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

Add DOM listeners before attaching the element #5195

Merged
merged 1 commit into from
Mar 7, 2019

Conversation

Legioth
Copy link
Member

@Legioth Legioth commented Mar 6, 2019

Fixes #5152


This change is Reviewable

@vaadin-bot
Copy link
Collaborator

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:

  1. MAJOR SimpleElementBindingStrategy.java#L195: Make the enclosing method "static" or remove this set. rule
  2. MAJOR SimpleElementBindingStrategy.java#L1005: Remove this unused private "invokeWhenNodeIsConstructed" method. rule
  3. MAJOR SimpleElementBindingStrategy.java#L1336: Equality tests should not be made with floating point values. rule
  4. INFO SimpleElementBindingStrategy.java#L195: Write to static field com.vaadin.client.flow.binding.SimpleElementBindingStrategy.boundNodes from instance method com.vaadin.client.flow.binding.SimpleElementBindingStrategy.bind(StateNode, Element, BinderContext) rule
  5. INFO SimpleElementBindingStrategy.java#L416: Complete the task associated to this TODO comment. rule

Copy link
Contributor

@denis-anisimov denis-anisimov left a 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: 1 unresolved discussion, 0 of 1 LGTMs obtained (waiting on @denis-anisimov)

a discussion (no related file):
I guess it's not even possible to add an HtmlUnit test for this, right ?


@Legioth
Copy link
Member Author

Legioth commented Mar 7, 2019

a discussion (no related file):

Previously, denis-anisimov (Denis) wrote…

I guess it's not even possible to add an HtmlUnit test for this, right ?

I couldn't come up with any way of testing this without a custom element that fires an event from connectedCallback. If I remember right, the web components polyfill doesn't work with HtmlUnit, which would indeed make it impossible to test it in this way.


Copy link
Contributor

@denis-anisimov denis-anisimov left a comment

Choose a reason for hiding this comment

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

Reviewable status: all discussions resolved, 0 of 1 LGTMs obtained

a discussion (no related file):

Previously, Legioth (Leif Åstrand) wrote…

I couldn't come up with any way of testing this without a custom element that fires an event from connectedCallback. If I remember right, the web components polyfill doesn't work with HtmlUnit, which would indeed make it impossible to test it in this way.

Yeah. I think that's the situation.


Copy link
Contributor

@denis-anisimov denis-anisimov left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! all discussions resolved, 1 of 1 LGTMs obtained

@denis-anisimov denis-anisimov merged commit 004c6e6 into master Mar 7, 2019
@denis-anisimov denis-anisimov deleted the addListenerBeforeAttach branch March 7, 2019 07:29
caalador pushed a commit that referenced this pull request Mar 7, 2019
denis-anisimov pushed a commit that referenced this pull request Mar 7, 2019
@denis-anisimov denis-anisimov added this to the 1.4.3 milestone Mar 7, 2019
manolo pushed a commit that referenced this pull request Mar 19, 2019
* Send service URL to the client side for Web Components UI (#5031)

Fixes #5044
Fixes #5027

* Remove dependency from data to html-components (#5050)

Fixes #5025

* Clarify StreamResource JavaDoc (#4981)

* 1.5-SNAPSHOT (#5055)

* Fix flow-component-renderer for Chrome 72 (#5060)

Fixes #5025, vaadin/vaadin-grid-flow#502

* Make the java doc return say the correct thing. (#5063)

* Shortcuts: change click shortcut's default behavior (#5058) (#5065)

* Deprecated @VaadinServletConfiguration (#5052)

* Update chrome driver version (#5020)

* Update byte buddy version (#5026)

* Update byte buddy version

Fixes #4956

* Fix version id in drivers (#5083)

* Deprecate getBodyAttributes and replace it with getHtmlAttributes (#5019) Fixes #4765

* Add IT test for push in embedded web components (#5059) Fixes #5006

* Add m2e lifecycle mapping for maven-antrun-plugin (#5087)

* Add m2e lifecycle mapping for maven-antrun-plugin

* Catch Throwable during maps sync handling (#5077)

* Shortcuts no longer work, when element is disabled (#5099)

* Shortcuts no longer work, when element is disabled

* Fix getChildCount to always use filter  (#5101) (Fixes vaadin-grid-flow/issues/464)

* Fix invalid null check (#5132)

* Javadoc - Add a component already added to a parent. (#5137)

* Document the fact that a component with a parent will be added to a new parent and removed from the previous one.

* Added Thread.sleeps to make sure Enter key is registered (#5139)

* flow-component-renderer: set focus-target attribute to the first focusable node (#5142)

Fixes #4191

Defining focus-target is necessary for the grid cell delegating focus to the first focusable component within it when Enter key is used (tests available from another PR on vaadin-grid-flow repo)

* Add some configuration for surefire plugin (#5144)

* Add some configuration for surefire plugin

Due to issues with tests run under Windows 10 we need to
add the trimStackTrace and reuseForks configurations also
to the surefire plugin

* Also update the surefire and failsafe plugins.

* Register push static web resources (#5112)

* Avoid for..of loop in renderer (#5171)

* Add DOM listeners before attaching the element (#5195)

Fixes #5152

* Add a generic mechanism for passing values back to the server (#5093)

This is an enabler for #1724 and #4925.

Fixes #4927

* Register superclass in case of route target collision (#5218)

Fixes #5114

* Introduce ValueChangeMode#LAZY, and #TIMEOUT (#4945)

Introduces ValueChangeMode#LAZY, and #TIMEOUT for delayed value synchronization to the server and makes Input implement HasValueChangeMode.

* Check the function presence before calling it (#5208)

Check the function presence before calling it and take care about children property as well.

Fixes #5206

* enable TrackMessageSizeInterceptor to deal with multiple messages in same response

* test for _trackMessageSize responsible to handle multiple messages coming from a single response

* refactor imports

* refactor TrackMessageSize test

* using IOUtils to read from InputStream

* Test for Long Polling when multiple threads run within a short period

* Enhanced code

* refactor getFileContent to use full path to retrieve vaadinPush.js

* Correct javadocs for DomEvent (#5215)

Fixes #5211

* Update jsoup version (#5253)

Fixes #4127

* Avoid indeterministic behavior in tests. (#5283)

Set locale explicitly in the tests.

Fixes #3500

* Handle empty attribute values properly in HTML element (#5255)

Fixes #5220

* Update chrome version in tests (#5295)

* Remove redundant words form javadocs (#5294)

Fixes #2770

* Update chromedriver version (#5297)

* Exporter-based embeddable web component impl. (#5260)

WebComponent is currently named IWebComponent to avoid naming collisions with
the existing classes - this will be fixed once the functionality has been
transferred over to the new components.

* Merge branch 'master' into issues/merge_from_master

# Conflicts:
#	build-tools/pom.xml
#	flow-bom/pom.xml
#	flow-client/pom.xml
#	flow-components-parent/flow-code-generator-api/pom.xml
#	flow-components-parent/flow-code-generator/pom.xml
#	flow-components-parent/flow-component-demo-helpers/pom.xml
#	flow-components-parent/flow-generated-components/pom.xml
#	flow-components-parent/flow-webcomponent-api-analyzer/pom.xml
#	flow-components-parent/pom.xml
#	flow-data/pom.xml
#	flow-data/src/test/java/com/vaadin/flow/data/binder/testcomponents/TestLabel.java
#	flow-html-components-testbench/pom.xml
#	flow-html-components/pom.xml
#	flow-maven-plugin/pom.xml
#	flow-osgi/pom.xml
#	flow-push/pom.xml
#	flow-server-production-mode/pom.xml
#	flow-server/pom.xml
#	flow-server/src/main/java/com/vaadin/flow/component/WebComponent.java
#	flow-server/src/main/java/com/vaadin/flow/component/webcomponent/WebComponentMethod.java
#	flow-server/src/main/java/com/vaadin/flow/server/communication/WebComponentProvider.java
#	flow-server/src/main/java/com/vaadin/flow/server/webcomponent/WebComponentGenerator.java
#	flow-server/src/test/java/com/vaadin/flow/internal/nodefeature/ReturnChannelMapTest.java
#	flow-server/src/test/java/com/vaadin/flow/server/BootstrapHandlerTest.java
#	flow-server/src/test/java/com/vaadin/flow/server/communication/WebComponentProviderTest.java
#	flow-server/src/test/java/com/vaadin/flow/server/communication/rpc/ReturnChannelHandlerTest.java
#	flow-server/src/test/java/com/vaadin/flow/server/webcomponent/WebComponentGeneratorTest.java
#	flow-test-generic/pom.xml
#	flow-test-util/pom.xml
#	flow-tests/pom.xml
#	flow-tests/servlet-containers/felix-jetty/pom.xml
#	flow-tests/servlet-containers/pom.xml
#	flow-tests/servlet-containers/tomcat85/pom.xml
#	flow-tests/servlet-containers/tomcat9/pom.xml
#	flow-tests/test-common/pom.xml
#	flow-tests/test-dev-mode/pom.xml
#	flow-tests/test-frontend-production-custom-context/pom.xml
#	flow-tests/test-lumo-theme/pom.xml
#	flow-tests/test-material-theme/pom.xml
#	flow-tests/test-memory-leaks/pom.xml
#	flow-tests/test-pwa/pom.xml
#	flow-tests/test-resources/pom.xml
#	flow-tests/test-root-context/pom.xml
#	flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/DomListenerOnAttachView.java
#	flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/ReturnChannelView.java
#	flow-tests/test-root-context/src/test/java/com/vaadin/flow/uitest/ui/ReturnChannelIT.java
#	flow-tests/test-root-ui-context/pom.xml
#	flow-tests/test-router-custom-context/pom.xml
#	flow-tests/test-scalability/pom.xml
#	flow-tests/test-servlet/pom.xml
#	flow-tests/test-subcontext/pom.xml
#	flow-tests/test-themes/pom.xml
#	flow-tests/test-web-components/pom.xml
#	flow-tests/test-web-components/src/main/java/com/vaadin/flow/webcomponent/ClientSelect.java
#	flow-tests/test-web-components/src/main/java/com/vaadin/flow/webcomponent/UpdateServerSideWebComponent.java
#	flow-tests/test-web-components/src/main/webapp/push.html
#	flow-tests/test-web-components/src/test/java/com/vaadin/flow/webcomponent/UpdateServerSideWebComponentIT.java
#	flow-theme-integrations/lumo/pom.xml
#	flow-theme-integrations/material/pom.xml
#	flow-theme-integrations/pom.xml
#	flow/pom.xml
#	pom.xml
@ZheSun88 ZheSun88 modified the milestones: 1.4.3, 1.5.0.alpha1 Mar 29, 2019
manolo pushed a commit that referenced this pull request Apr 8, 2019
* Send service URL to the client side for Web Components UI (#5031)

Fixes #5044
Fixes #5027

* Remove dependency from data to html-components (#5050)

Fixes #5025

* Clarify StreamResource JavaDoc (#4981)

* 1.5-SNAPSHOT (#5055)

* Fix flow-component-renderer for Chrome 72 (#5060)

Fixes #5025, vaadin/vaadin-grid-flow#502

* Make the java doc return say the correct thing. (#5063)

* Shortcuts: change click shortcut's default behavior (#5058) (#5065)

* Deprecated @VaadinServletConfiguration (#5052)

* Update chrome driver version (#5020)

* Update byte buddy version (#5026)

* Update byte buddy version

Fixes #4956

* Fix version id in drivers (#5083)

* Deprecate getBodyAttributes and replace it with getHtmlAttributes (#5019) Fixes #4765

* Add IT test for push in embedded web components (#5059) Fixes #5006

* Add m2e lifecycle mapping for maven-antrun-plugin (#5087)

* Add m2e lifecycle mapping for maven-antrun-plugin

* Catch Throwable during maps sync handling (#5077)

* Shortcuts no longer work, when element is disabled (#5099)

* Shortcuts no longer work, when element is disabled

* Fix getChildCount to always use filter  (#5101) (Fixes vaadin-grid-flow/issues/464)

* Fix invalid null check (#5132)

* Javadoc - Add a component already added to a parent. (#5137)

* Document the fact that a component with a parent will be added to a new parent and removed from the previous one.

* Added Thread.sleeps to make sure Enter key is registered (#5139)

* flow-component-renderer: set focus-target attribute to the first focusable node (#5142)

Fixes #4191

Defining focus-target is necessary for the grid cell delegating focus to the first focusable component within it when Enter key is used (tests available from another PR on vaadin-grid-flow repo)

* Add some configuration for surefire plugin (#5144)

* Add some configuration for surefire plugin

Due to issues with tests run under Windows 10 we need to
add the trimStackTrace and reuseForks configurations also
to the surefire plugin

* Also update the surefire and failsafe plugins.

* Register push static web resources (#5112)

* Avoid for..of loop in renderer (#5171)

* Add DOM listeners before attaching the element (#5195)

Fixes #5152

* Add a generic mechanism for passing values back to the server (#5093)

This is an enabler for #1724 and #4925.

Fixes #4927

* Register superclass in case of route target collision (#5218)

Fixes #5114

* Introduce ValueChangeMode#LAZY, and #TIMEOUT (#4945)

Introduces ValueChangeMode#LAZY, and #TIMEOUT for delayed value synchronization to the server and makes Input implement HasValueChangeMode.

* Check the function presence before calling it (#5208)

Check the function presence before calling it and take care about children property as well.

Fixes #5206

* enable TrackMessageSizeInterceptor to deal with multiple messages in same response

* test for _trackMessageSize responsible to handle multiple messages coming from a single response

* refactor imports

* refactor TrackMessageSize test

* using IOUtils to read from InputStream

* Test for Long Polling when multiple threads run within a short period

* Enhanced code

* refactor getFileContent to use full path to retrieve vaadinPush.js

* Correct javadocs for DomEvent (#5215)

Fixes #5211

* Update jsoup version (#5253)

Fixes #4127

* Avoid indeterministic behavior in tests. (#5283)

Set locale explicitly in the tests.

Fixes #3500

* Handle empty attribute values properly in HTML element (#5255)

Fixes #5220

* Update chrome version in tests (#5295)

* Remove redundant words form javadocs (#5294)

Fixes #2770

* Update chromedriver version (#5297)

* Exporter-based embeddable web component impl. (#5260)

WebComponent is currently named IWebComponent to avoid naming collisions with
the existing classes - this will be fixed once the functionality has been
transferred over to the new components.

* Fixed a NPE problem in WebComponentConfigurationRegistry (#5304)

* Implement open and setLocation method to Page. (#4869) (#5166)

* Fix navigate back to main view don't work in FF and Safari . (#5113) (#5291)

* Register a servlet automatically if WCs present (#5331)

Fixes #5070

* EWC: push property updates to client (#5324)

* Apply theme for web component exporters (#5341)

Fix for #4984

* Only make banner the target of pointer events (#5343)

Currently the prompt consists of a full width transparent `#pwa-ip` div which is preventing click in elements behind it, and a `.banner` div which should be clickable

* WebComponent fireEvent implementation (#5340)

Implements #5272

* Fixes #5351 (#5358)

* Detect Push from WebComponentExporter classes (#5369), Fix for #4984

* Make exported web components work on hosts where https is proxied to http (#5374)

* Update config and generated class for rich-text-editor (#4754)

* Merge InstanceConfigurator with exporter (#5376) (Fixes #5332)

Also: Ignore abstract class in exporters discovering logic

* Use "create" semantic for the createComponent method (#5379)

Fixes #4634

* Avoid "web context" in web component module generation (#5391)

* Avoid "web context" in web component module generation

Fixes #5382

* Correct unit test

* Improve JS code

* Extract web component config factory (#5403)

Fixes #5402

* Correct generated template to work properly in production mode (#5421)

* Generate web component module file in the maven plugin to be able to transpile it (#5419)

* Initial preparation for web component module files generation

* Generate web component module content and add it into the bundle for
transpilation

* Generate web component module content and add the generated file into
the bundle file

* Hide common generation method

* Correct typos.

* Merge master into 2.0

Resolve the merge conflicts.

* Remove empty @SInCE javadocs

* Remove unused class

* Fix the test
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.

None yet

4 participants