Only clear all if value changes. #7212

caalador · 2019-12-20T08:50:03Z

If we remove all when the innerHTML
value stays the same we will loose the innerHTML
on the client as the value is not re-sent,
but is still cleared from the client.

Fixes #4644

This change is

If we remove all when the innerHTML value stays the same we will loose the innerHTML on the client as the value is not re-sent, but is still cleared from the client. Fixes #4644

denis-anisimov

Reviewed 3 of 3 files at r1.
Reviewable status: 1 unresolved discussion, 0 of 1 LGTMs obtained (waiting on @denis-anisimov)

a discussion (no related file):
This also should be tested via unit test.
May be it's even better to have only a unit test.
But the IT is already added, so let it be.

caalador

Reviewable status: 1 unresolved discussion, 0 of 1 LGTMs obtained (waiting on @denis-anisimov)

a discussion (no related file):

Previously, denis-anisimov (Denis) wrote…

This also should be tested via unit test.
May be it's even better to have only a unit test.
But the IT is already added, so let it be.

I was going to do a unit test, but the problem is that the remove all doesn't on the server clear the property as it is not a child, but on the client innerHTML is seen as a child element and is removed there.

denis-anisimov

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

a discussion (no related file):

Previously, caalador wrote…

I was going to do a unit test, but the problem is that the remove all doesn't on the server clear the property as it is not a child, but on the client innerHTML is seen as a child element and is removed there.

Ah, alright.

vaadin-bot · 2019-12-20T09:41:33Z

SonarQube analysis reported 1 issue

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:

Element.java#L1841: Remove useless curly braces around statement

* Avoid clean up if PNPM is used (#7095) Fixes #7083 * Iterate over a copy for UI internals listeners (#7103) Avoids ConcurrentModificationException if firing an event causes listeners to be added or removed. * Convert lambda exps to simple loops to fix Java 11 build (#7102) * Fix command expectations to expect a local install (#7100) Test run against a local installation of pnpm test was expecting a global installation to be used. * Highlight drop target when dragged on child Fixes #7108 ..but does not fix case where child is a drop target too and drag moves back to parent drop target -> the class name for parent element is not reapplied. * Make nested drop target highlighting work Fixes #7109 * Allow adding a local js module through Page.addJsModule (#6933) * Deny adding a local js module through Page.addJsModule * Deprecate Page.addJsModule(url, loadMode) * Remove old "flow-deps" dependency/package-lock if PNPM is used (#7117) Part of #7113 * Fix using host value from the request (#7115) When using the host value from the request header we need to add the transport as host only contains the server and port. In the off case that we get the scheme as a header we should use that. * Exclude pnpm from managing itself (#7125) * Use recommended API to get chrome capabilities (#7138) DesiredCapabilities.chrome will be deprecated and removed. Current usage results in logged warnings "Using `new ChromeOptions()` is preferred to `DesiredCapabilities.chrome()`" https://github.com/SeleniumHQ/selenium/blob/selenium-3.141.59/java/client/src/org/openqa/selenium/remote/DesiredCapabilities.java#L115-L118 * Add explicit "crossorigin" attribute to bundle script tag in bootstrap (#7124) * Use Element API for traversing children to call the onEnabledStateChanged method (#7131) Fixes #7085 * Deprecate isIOS method and make it in ExtendedClientDetails (#7139) Fixes #6671 * Added missing constructor with scanNestedDefinitions option (#7099) Cherry pick from: vaadin/framework#11801 * Upgrade webpack plugins to fix npm audit warnings (#7122) Upgrade compression-webpack-plugin dependency Upgrade copy-webpack-plugin version Upgrade webpack dependencies versions * Only sending to server should be postponed - Update value in StateTree on the client side immediately - Defer execution only sending a new value to the server Fixes #7128 * Add IT test * Remove IT test which doesn't reproduce the issue * Handle possible array (#7159) If building sourcemaps for webpack the assets array will contain an array of bundles instead of a single string. Fixes #7158 * Workarounds for Safari 10 <script nomodule> and scope bugs (#7165) * Unregister listener when disconnecting web component (#7061) * Make links focusable (#7155) * Fix broken mobile dnd polyfill for ipad on iOS13 Fixes #7123 * Copy when jar (#7175) If packaging the project as a jar then we will copy the frontend files during prepare-frontend. This way running the development mode jar should have all frontend files available. Fixes #6994 * Fix running pnpm for a local installation (#7185) * Fix ElementStateProvider serialization (#7192) Add readResolve method for singleton class handing during (de)serialization of state providers. Fixes #7190 * Pin pnpm verion to 4.5 and remove workaround for 4.3.3 (#7191) Fixes #7183 * Pass query parameters to the new location (#7189) * Update chromedriver to v79 (#7181) * Assert chrome version 79 (#7201) * Only clear all if value changes. (#7212) If we remove all when the innerHTML value stays the same we will loose the innerHTML on the client as the value is not re-sent, but is still cleared from the client. Fixes #4644 * Don't add abstract error targets (#7215) Filter out any abstract classes from error navigation targets as they can't be instantiated anyway. Fixes vaadin/spring#530 * Add onclick to error window (#7217) Now the webpack error window can be closed by clicking it. Fixes #7211 * Error message for failing imports should be tool dependent (#7216) Fixes #7182 * Combine all constructors together Fixes #7148 * log: debug the list of dependencies and imports found in classes (#7221) Co-authored-by: Denis <denis@vaadin.com> Co-authored-by: Leif Åstrand <legioth@gmail.com> Co-authored-by: Mehdi Javan <32511762+mehdi-vaadin@users.noreply.github.com> Co-authored-by: caalador <mikael.grankvist@gmail.com> Co-authored-by: Pekka Hyvönen <pekka@vaadin.com> Co-authored-by: Bogdan Udrescu <bogdanudrescu@users.noreply.github.com> Co-authored-by: Guille <alvarezguille@users.noreply.github.com> Co-authored-by: Johannes Eriksson <joheriks@vaadin.com> Co-authored-by: Serhii Kulykov <iamkulykov@gmail.com> Co-authored-by: Muammer Yucel <muammer.yucel.82@gmail.com> Co-authored-by: Giovanni Lovato <giovanni@lova.to> Co-authored-by: Manuel Carrasco Moñino <manolo@apache.org>

If we remove all when the innerHTML value stays the same we will loose the innerHTML on the client as the value is not re-sent, but is still cleared from the client. Fixes #4644 (cherry picked from commit efdc44a)

* Support for incorrect webjar paths #4601 (#5951) Add support to the StaticFileServer for understanding incorrect webjar paths generated by the production build. (cherry picked from commit b70c575) * Use Element API for traversing children to call the onEnabledStateChanged method (#7131) Fixes #7085 (cherry picked from commit c3f6012) * Update chromedriver to v79 (#7181) (cherry picked from commit debbbc8) * Only clear all if value changes. (#7212) If we remove all when the innerHTML value stays the same we will loose the innerHTML on the client as the value is not re-sent, but is still cleared from the client. Fixes #4644 (cherry picked from commit efdc44a)

Only clear all if value changes.

c3b9e7a

If we remove all when the innerHTML value stays the same we will loose the innerHTML on the client as the value is not re-sent, but is still cleared from the client. Fixes #4644

denis-anisimov suggested changes Dec 20, 2019

View reviewed changes

caalador commented Dec 20, 2019

View reviewed changes

denis-anisimov approved these changes Dec 20, 2019

View reviewed changes

vaadin-bot added the +0.0.1 label Dec 20, 2019

caalador merged commit efdc44a into master Dec 20, 2019

caalador deleted the issues/4644_innerHTML branch December 20, 2019 10:32

caalador added the to be cherry picked label Dec 20, 2019

mehdi-vaadin removed the to be cherry picked label Jan 2, 2020

mehdi-vaadin added the cherry picked label Jan 2, 2020

joheriks added this to the 2.2.0.alpha9 milestone Jan 9, 2020

mehdi-vaadin added a commit that referenced this pull request Jan 9, 2020

Revert #7212 as its IT fails in Flow 1.0

21db2d5

mehdi-vaadin mentioned this pull request Jan 9, 2020

Revert #7212 as its IT fails in Flow 1.0 #7319

Merged

mehdi-vaadin added a commit that referenced this pull request Jan 13, 2020

Revert #7212 as its IT fails in Flow 1.0 (#7319)

0199f1d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Only clear all if value changes. #7212

Only clear all if value changes. #7212

caalador commented Dec 20, 2019 •

edited by mehdi-vaadin

Loading

denis-anisimov left a comment

caalador left a comment

denis-anisimov left a comment

vaadin-bot commented Dec 20, 2019

Only clear all if value changes. #7212

Only clear all if value changes. #7212

Conversation

caalador commented Dec 20, 2019 • edited by mehdi-vaadin Loading

denis-anisimov left a comment

Choose a reason for hiding this comment

caalador left a comment

Choose a reason for hiding this comment

denis-anisimov left a comment

Choose a reason for hiding this comment

vaadin-bot commented Dec 20, 2019

caalador commented Dec 20, 2019 •

edited by mehdi-vaadin

Loading