-
Notifications
You must be signed in to change notification settings - Fork 168
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
Use InitialPropertiesHandler for injected elements #4454
Use InitialPropertiesHandler for injected elements #4454
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 4 of 4 files at r1.
Reviewable status: 2 unresolved discussions, 0 of 1 LGTMs obtained
a discussion (no related file):
Maybe this bugfix will also fix vaadin/vaadin-grid-flow#71
flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/KeyPressListenerView.java, line 39 at r1 (raw file):
field.addKeyPressListener(Key.ENTER, event -> { div.setText("enter"); ;
Extra ;
here. In fact for this lambda you can even remove the curly bracers.
flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/KeyPressListenerView.java, line 42 at r1 (raw file):
}); field.addKeyPressListener(Key.ARROW_DOWN, event -> { System.out.println("arrow-down");
Shouldn't it be div.setText("arrow-down")
instead?
47cf118
to
b7cae0e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 2 unresolved discussions, 0 of 1 LGTMs obtained
flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/KeyPressListenerView.java, line 39 at r1 (raw file):
Previously, gilberto-torrezan (Gilberto Torrezan) wrote…
Extra
;
here. In fact for this lambda you can even remove the curly bracers.
This is leftover from unrelated investigation.
Removed.
flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/KeyPressListenerView.java, line 42 at r1 (raw file):
Previously, gilberto-torrezan (Gilberto Torrezan) wrote…
Shouldn't it be
div.setText("arrow-down")
instead?
This is leftover from unrelated investigation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r2.
Reviewable status: all discussions resolved, 0 of 1 LGTMs obtained, and 1 stale
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! all discussions resolved, 1 of 1 LGTMs obtained
SonarQube analysis reported 5 issues Note: The following issues were found on lines that were not modified in the pull request. Because these issues can't be reported as line comments, they are summarized here:
|
Fixes #4304
This change is