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

Fix single selection on the same row, item updates #2877

Merged
merged 6 commits into from
Nov 9, 2017

Conversation

ahie
Copy link
Contributor

@ahie ahie commented Nov 8, 2017

Fixes #2874
Fixes #2872


This change is Reviewable

@gilberto-torrezan
Copy link
Contributor

Reviewed 2 of 2 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


flow-components-parent/demo-flow-components/src/test/java/com/vaadin/flow/demo/views/GridViewIT.java, line 112 at r1 (raw file):

                messageDiv.getText());

        Assert.assertFalse(isElementPresent(By.className("v-system-error")));

Is this reliable?
(I asked about this on Slack)
If at some point the box message changes the class name, this test will be green even when a message is shown.
But I don't know a better way of doing this (hence the question on Slack)


Comments from Reviewable

@gilberto-torrezan
Copy link
Contributor

Review status: all files reviewed at latest revision, 1 unresolved discussion.


flow-components-parent/demo-flow-components/src/test/java/com/vaadin/flow/demo/views/GridViewIT.java, line 112 at r1 (raw file):

Previously, gilberto-torrezan (Gilberto Torrezan) wrote…

Is this reliable?
(I asked about this on Slack)
If at some point the box message changes the class name, this test will be green even when a message is shown.
But I don't know a better way of doing this (hence the question on Slack)

Kirill answered with a code that inspects the error messages in the client console. That is more reliable, and could be exported as an Util method for other tests that need the same functionality.


Comments from Reviewable

@ahie
Copy link
Contributor Author

ahie commented Nov 9, 2017

Review status: 1 of 3 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


flow-components-parent/demo-flow-components/src/test/java/com/vaadin/flow/demo/views/GridViewIT.java, line 112 at r1 (raw file):

Previously, gilberto-torrezan (Gilberto Torrezan) wrote…

Kirill answered with a code that inspects the error messages in the client console. That is more reliable, and could be exported as an Util method for other tests that need the same functionality.

Done.


Comments from Reviewable

@gilberto-torrezan
Copy link
Contributor

Reviewed 2 of 2 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@gilberto-torrezan
Copy link
Contributor

Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


flow-components-parent/demo-flow-components/src/test/java/com/vaadin/flow/demo/views/GridViewIT.java, line 112 at r2 (raw file):

                messageDiv.getText());

        Assert.assertFalse(getLogErrors().findAny().isPresent());

The problem in CI might be that the code also gets all warnings, and you just want errors.


Comments from Reviewable

@gilberto-torrezan
Copy link
Contributor

Reviewed 2 of 2 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@denis-anisimov denis-anisimov merged commit 50a5f8c into master Nov 9, 2017
@denis-anisimov denis-anisimov deleted the grid-selection-fix branch November 9, 2017 09:59
@denis-anisimov denis-anisimov added this to the 1.0.0.alpha9 milestone Nov 9, 2017
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.

3 participants