Don't override initial value of hidden HTML Element attribute.

[denis-anisimov]

Fixes #8100

---

This change is 

[joheriks]

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

---

flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java, line 589 at r1 (raw file):

        NodeMap visibilityData = context.node.getMap(NodeFeatures.ELEMENT_DATA);
        // Store the current "hidden" value to restore it when the element
        // becomes visible

Should this comment be removed as it pertained to the removed line?

---

flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/RemoveAddVisibilityView.java, line 24 at r1 (raw file):

        add(toggle, hidden);
    }
}

• I would prefer for simplicity if the button only set the element visible (instead of toggling), as there is anyway no test for going back to invisible (unlike the other IT). Less code and mutable state makes IT easier to understand and modify if needed.
• Add a newline at the end.

[denis-anisimov]

Reviewable status: 2 unresolved discussions, 0 of 1 LGTMs obtained (waiting on @joheriks)

---

flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java, line 589 at r1 (raw file):

Previously, joheriks (Johannes Eriksson) wrote…

Should this comment be removed as it pertained to the removed line?

Good catch.
Forgot about the comment after several rewritings.

---

flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/RemoveAddVisibilityView.java, line 24 at r1 (raw file):

Previously, joheriks (Johannes Eriksson) wrote…

• I would prefer for simplicity if the button only set the element visible (instead of toggling), as there is anyway no test for going back to invisible (unlike the other IT). Less code and mutable state makes IT easier to understand and modify if needed.
• Add a newline at the end.

Well, it makes sense.
The code snippet is almost exact copy of the code from the ticket.
But you are right: there is no sense to make it toggle.

[denis-anisimov]

Reviewable status: 2 unresolved discussions, 0 of 1 LGTMs obtained (waiting on @joheriks)

---

flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java, line 589 at r1 (raw file):

Previously, denis-anisimov (Denis) wrote…

Good catch.
Forgot about the comment after several rewritings.

Done.

---

flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/RemoveAddVisibilityView.java, line 24 at r1 (raw file):

Previously, denis-anisimov (Denis) wrote…

Well, it makes sense.
The code snippet is almost exact copy of the code from the ticket.
But you are right: there is no sense to make it toggle.

Done.

[joheriks]

Reviewed 3 of 3 files at r2.
Reviewable status: complete! all discussions resolved, 1 of 1 LGTMs obtained