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

Update BaseChosenValueListBox.java #242

Merged
merged 14 commits into from
Oct 19, 2015
Merged

Update BaseChosenValueListBox.java #242

merged 14 commits into from
Oct 19, 2015

Conversation

foal
Copy link
Contributor

@foal foal commented Jul 25, 2015

isAccepted method of BaseChosenValueListBox check against value #241

isAccepted method of BaseChosenValueListBox check against value #241
@olafleur
Copy link
Member

Could you provide a test that would fail before your change (and pass after)?
That would really help.

Thanks for your contribution!

@foal
Copy link
Contributor Author

foal commented Aug 5, 2015

Sorry I spent more than one hour to try find right combination of a Chrome and chrome driver :( The mvn gwt:run -Pintegration-test does not work too (Failed to execute goal org.codehaus.mojo:gwt-maven-plugin:2.7.0:run (default-cli) on project integration-test: The parameters 'runTarget' for goal org.codehaus.mojo:gwt-maven-plugin:2.7.0:run are missing or invalid)

I think you can ease reproduce the error if update the listBox in SimpleValueListBox with custom key provider.

        listBox = new ChosenValueListBox<CarBrand>(getRenderer(), new ProvidesKey<CarBrand>() {
            @Override
            public Object getKey(final CarBrand item) {
                return item.name();
            }
        }, options);

@foal
Copy link
Contributor Author

foal commented Aug 5, 2015

I commit proposed update. Something wrong with check stile report generation....

@foal
Copy link
Contributor Author

foal commented Aug 10, 2015

Rollback the test

@Pacane
Copy link
Contributor

Pacane commented Oct 14, 2015

LGTM, thanks for the contribution!

@@ -0,0 +1,84 @@
/**
* Copyright 2014 ArcBees Inc.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2015

return listBox;
}

protected Renderer<CarBrand> getRenderer() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need this method, simply use RENDERER where needed

meriouma added a commit that referenced this pull request Oct 19, 2015
Update BaseChosenValueListBox.java
@meriouma meriouma merged commit 81c336a into ArcBees:master Oct 19, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants