Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MaterialComboBox.setSingleValue fires ValueChangeEvent with wrong values #291

Closed
gkresic opened this issue Dec 27, 2017 · 4 comments
Closed

Comments

@gkresic
Copy link
Contributor

gkresic commented Dec 27, 2017

Looking at MaterialComboBox.setSingleValue(T, boolean) isn't it wrong to use values as new value (values contains all possible items in combo, not newly selected one)? Shouldn't it be something like Collections.singletonList(value)?

@kevzlou7979 kevzlou7979 self-assigned this Jan 19, 2018
@kevzlou7979 kevzlou7979 added this to the 2.0.1 milestone Jan 19, 2018
@kevzlou7979
Copy link
Contributor

kevzlou7979 commented Feb 1, 2018

I've tried to update the method you proposed but it causing a major issue on ComboBox now.

public void setSingleValue(T value, boolean fireEvents) {
        int index = this.values.indexOf(Collections.singletonList(value));
        if (index >= 0) {
            List<T> before = getValue();
            setSelectedIndex(index);

            if (fireEvents) {
                ValueChangeEvent.fireIfNotEqual(this, before, values);
            }
        }
    }

@gkresic
Copy link
Contributor Author

gkresic commented Feb 1, 2018

It's been a while, but if I remember correctly, change was not necessary in #471:

int index = this.values.indexOf(value);

but at #477 where event fires:

ValueChangeEvent.fireIfNotEqual(this, before, Collections.singletonList(value));

@kevzlou7979
Copy link
Contributor

Oops yeah, thanks for this update. I will try to check if something is breaking up on unit tests. If there's nothing then I can patch this up to 2.0.1

kevzlou7979 added a commit that referenced this issue Feb 1, 2018
@kevzlou7979
Copy link
Contributor

Updated the fix f0fbb27

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants