Request: Implement `select_by_value` for `select` elements about fantoccini HOT 10 CLOSED

Unfortunately I have run out of time for this and have been moved onto other projects that currently do not require this functionality meaning this issue is up-for-grabs for someone else to take a stab at.

from fantoccini.

Status update:

I have resolved the Rc issue (I do not know if my approach is correct though) but am on to type-mismatch issues.

impl<'c> Select<'c> {
    /// TODO: docs
    pub fn from(c: &'c Client, e: Element) -> Self {
        Self { c, e }
    }

    /// TODO: docs
    pub fn select_by_value(self, value: &'c str) -> impl Future<Item = Client, Error = error::CmdError> + 'c {
        let locator = format!("option[value=\"{}\"]", value); // TODO: Escape `value`
        let locator = webdriver::command::LocatorParameters {
            using: webdriver::common::LocatorStrategy::CSSSelector,
            value: locator,
        };

        // TODO: Use FindElementElements
        let cmd = WebDriverCommand::FindElementElement(self.e.e, locator);
        Box::new(self.c.dup().issue_wd_cmd(cmd)
            .map_err(|e| e.into())
            .and_then(move |(c, res): (Client, rustc_serialize::json::Json)| {
                let e = c.parse_lookup(res);
                e.map(move |e| Element { c, e })
            }).and_then(move |e: Element| {
                e.is_selected()
                .map(|val: bool| {
                    if !val {
                        let f = e.click();
                        futures::future::Either::A(f) 
                    } else {
                        let f = futures::future::ok::<Client, error::CmdError>(e.c);
                        futures::future::Either::B(f)
                    }
                })
            })) as Box<Future<Item = _, Error = _>>
    }
}

error[E0271]: type mismatch resolving `<[closure@/Users/thill/src/experiments/fantoccini/src/lib.rs:330:22: 338:18 e:Element] as std::ops::FnOnce<(bool,)>>::Output == Client`
   --> /Users/thill/src/experiments/fantoccini/src/lib.rs:323:9
    |
323 | /         Box::new(self.c.dup().issue_wd_cmd(cmd)
324 | |             .map_err(|e| e.into())
325 | |             .and_then(move |(c, res): (Client, rustc_serialize::json::Json)| {
326 | |                 let e = c.parse_lookup(res);
...   |
338 | |                 })
339 | |             })) as Box<Future<Item = _, Error = _>>
    | |_______________^ expected enum `futures::future::Either`, found struct `Client`
    |
    = note: expected type `futures::future::Either<impl futures::Future, futures::FutureResult<Client, error::CmdError>>`
               found type `Client`
    = note: required because of the requirements on the impl of `futures::Future` for `futures::Map<impl futures::Future, [closure@/Users/thill/src/experiments/fantoccini/src/lib.rs:330:22: 338:18 e:Element]>`
    = note: required for the cast to the object type `futures::Future<Item=Client, Error=error::CmdError>`

Given Element::is_selected is:

    pub fn is_selected(self) -> impl Future<Item = bool, Error = error::CmdError> + 'static {
        let cmd = WebDriverCommand::IsSelected(self.e);
        self.c.issue_wd_cmd(cmd).and_then(move |(_unk, res)| {
            println!("{:?}", res);
            Ok(true)
        })
    }

from fantoccini.

I'm not entirely sure why the is_selected branch is necessary? Couldn't you always just click the option? I also wonder if we should just fetch all the options under the select and do the checking for value in code. It'll save us the escaping of the value in the CSS selector, at the cost fo fetching more options (which probably doesn't matter all that much). Thoughts?

from fantoccini.

If you always click an option you may be deselecting an already clicked option in a multi-select dropdown.

I do like the sound of fetching all options up front and doing comparisons in our code. Performance evaluations needed of course but the idea doesn't hurt.

from fantoccini.

Yeah, I think I'd accept something like this. I'm a little tight for time in the coming weeks, but would be happy to take a look at and mentor a PR!

from fantoccini.

I found myself revisiting this mentally and think I just now understood a point you made @jonhoo.

I was previously under the assumption that click would deselect currently-selected options (which may be the case, I have not verified this one way or another) but if that is not the case then just a single click will work which means that the is_selected branch is indeed unnecessary.

If a click does deselect then perhaps setting the selected attr explicitly will be "good enough."

from fantoccini.

Another issue (which is mentioned here) is that a click may navigate.

This leads to the question should select_by_* functions:

• set the selected attr (which won't cause navigation), or
• use click

The latter is what a human user would experience so is probably preferred but we should make a conscious decision here.

Edit:

It should be noted that setting attributes is not part of the WD spec which means we'd have to execute JS on the client to do so (which is less than desirable).

from fantoccini.

Those are all good questions, and I don't really have an answer. I suspect the best way to deal with this is similar to how we distinguish between Element::click and Element::follow. One emulates a real user click, while the other deals directly with attributes. The various Form::submit_* methods are the same. Having two different accessors that implement the two behaviors you outline seems like a decent solution :)

FWIW: I'm fairly sure that clicking an already selected option will not unselect it (unlike, e.g., <input type="checkbox">).

from fantoccini.

I didn't articulate that well but what I meant was in a dropdown with multiple options if you have A selected then click B that A will be unselected (unless it is a multi-select and the user initiates a multi-select action [shift+click]).

from fantoccini.

Ohh, I see what you mean. I think it's fine for our click semantics to match those of a human clicking the same option (i.e., also unselecting other things). We could then have a select_option method that instead just marks the corresponding option as selected.

from fantoccini.