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

Parse template initial attribute values as properties, not attributes #8974

Closed
pleku opened this issue Sep 8, 2020 · 2 comments · Fixed by #8988
Closed

Parse template initial attribute values as properties, not attributes #8974

pleku opened this issue Sep 8, 2020 · 2 comments · Fixed by #8988

Comments

@pleku
Copy link
Contributor

pleku commented Sep 8, 2020

Follow up on #8957, given template with contents

<vaadin-text-field id="name" placeholder="John Doe" label="Name" autofocus max-lenght="30"></vaadin-text-field>

the initial attribute for the element should be mapped as properties*. Thus

@Id("name") TextField nameField;

nameField.getPlaceHolder(); // returns "John Doe"
nameField.getLabel(); // returns "Name"
nameField.isAutoFocus(); // returns true
nameField.getMaxLenght(); // 30

As can be seen for instance with https://vaadin.com/components/vaadin-text-field/html-api/elements/Vaadin.TextFieldElement the web component API has properties, but not attributes. So this needs to be the default.

*The special cases that are not properties are at least:

@denis-anisimov
Copy link
Contributor

This ticket may have dramatic impact on the templates behavior .

With the current implementation (exactly as described) the following happens:

  • template has an element with hidden : <div id='injected' hidden></div>.
  • hidden is read as a property (it's not in the list , so it's not read as an attribute EVEN though this is an attribute by HTML spec).
  • Flow treats empty values for attribute so that if it has "" value then its value is true ( and as a result it will be rendered as hidden) otherwise it will be removed. And hidden attribute will work correctly. But hidden property doesn't work since property value is set to "" and send to the client side where it's set to the same value. BUT this is not a property. This is an attribute. And HTML removes the attribute from the DOM .

What we should do : treat hidden as an attribute and not a property.
And it's obvious that hidden is just a special case. If some other attribute with boolean semantic is treated incorrectly as a property then we are introducing bugs.

@denis-anisimov
Copy link
Contributor

Should this be used as a reference for global attributes? :
https://www.w3schools.com/tags/ref_standardattributes.asp

I'm going to add hidden as an attribute otherwise there is an IT test which just fails because of that.

OLD Vaadin Flow ongoing work (Vaadin 10+) automation moved this from In progress to Done - pending release Sep 14, 2020
denis-anisimov pushed a commit that referenced this issue Sep 14, 2020
Read template attributes as properties

Fixes #8974
caalador pushed a commit that referenced this issue Jan 27, 2021
Read template attributes as properties

Fixes #8974
# Conflicts:
#	flow-server/src/main/java/com/vaadin/flow/component/littemplate/LitTemplateDataAnalyzer.java
#	flow-server/src/main/java/com/vaadin/flow/component/polymertemplate/IdCollector.java
#	flow-server/src/main/java/com/vaadin/flow/component/polymertemplate/TemplateDataAnalyzer.java
#	flow-server/src/main/java/com/vaadin/flow/component/polymertemplate/TemplateInitializer.java
#	flow-server/src/test/java/com/vaadin/flow/component/polymertemplate/PolymerTemplateTest.java
#	flow-tests/test-root-context/frontend/AttributeTemplate.js
#	flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/template/TemplateAttributeView.java
#	flow-tests/test-root-context/src/test/java/com/vaadin/flow/uitest/ui/template/TemplateAttributeIT.java
caalador pushed a commit that referenced this issue Jan 29, 2021
Read template attributes as properties

Fixes #8974
# Conflicts:
#	flow-server/src/main/java/com/vaadin/flow/component/littemplate/LitTemplateDataAnalyzer.java
#	flow-server/src/main/java/com/vaadin/flow/component/polymertemplate/IdCollector.java
#	flow-server/src/main/java/com/vaadin/flow/component/polymertemplate/TemplateDataAnalyzer.java
#	flow-server/src/main/java/com/vaadin/flow/component/polymertemplate/TemplateInitializer.java
#	flow-server/src/test/java/com/vaadin/flow/component/polymertemplate/PolymerTemplateTest.java
#	flow-tests/test-root-context/frontend/AttributeTemplate.js
#	flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/template/TemplateAttributeView.java
#	flow-tests/test-root-context/src/test/java/com/vaadin/flow/uitest/ui/template/TemplateAttributeIT.java
caalador pushed a commit that referenced this issue Feb 2, 2021
Read template attributes as properties

Fixes #8974
# Conflicts:
#	flow-server/src/main/java/com/vaadin/flow/component/littemplate/LitTemplateDataAnalyzer.java
#	flow-server/src/main/java/com/vaadin/flow/component/polymertemplate/IdCollector.java
#	flow-server/src/main/java/com/vaadin/flow/component/polymertemplate/TemplateDataAnalyzer.java
#	flow-server/src/main/java/com/vaadin/flow/component/polymertemplate/TemplateInitializer.java
#	flow-server/src/test/java/com/vaadin/flow/component/polymertemplate/PolymerTemplateTest.java
#	flow-tests/test-root-context/frontend/AttributeTemplate.js
#	flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/template/TemplateAttributeView.java
#	flow-tests/test-root-context/src/test/java/com/vaadin/flow/uitest/ui/template/TemplateAttributeIT.java
pleku pushed a commit that referenced this issue Feb 2, 2021
Read template attributes as properties

Fixes #8974
# Conflicts:
#	flow-server/src/main/java/com/vaadin/flow/component/littemplate/LitTemplateDataAnalyzer.java
#	flow-server/src/main/java/com/vaadin/flow/component/polymertemplate/IdCollector.java
#	flow-server/src/main/java/com/vaadin/flow/component/polymertemplate/TemplateDataAnalyzer.java
#	flow-server/src/main/java/com/vaadin/flow/component/polymertemplate/TemplateInitializer.java
#	flow-server/src/test/java/com/vaadin/flow/component/polymertemplate/PolymerTemplateTest.java
#	flow-tests/test-root-context/frontend/AttributeTemplate.js
#	flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/template/TemplateAttributeView.java
#	flow-tests/test-root-context/src/test/java/com/vaadin/flow/uitest/ui/template/TemplateAttributeIT.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
OLD Vaadin Flow ongoing work (Vaadin ...
  
Done - pending release
Development

Successfully merging a pull request may close this issue.

2 participants