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

Improve @Id documentation to indicate possible overriding of attributes #10668

Closed
tarekoraby opened this issue Apr 15, 2021 · 3 comments · Fixed by #10802
Closed

Improve @Id documentation to indicate possible overriding of attributes #10668

tarekoraby opened this issue Apr 15, 2021 · 3 comments · Fixed by #10802

Comments

@tarekoraby
Copy link
Contributor

Please consider improving the documentation of the @Id annotation to indicate that its use could override the attribute values of the bound element that are set in the Template.

For example, if one sets the orientation of a split-layout in a Template:

<vaadin-split-layout orientation="vertical" id="layout">
  <div>primary</div>
  <div>secondary</div>
</vaadin-split-layout>

Then in Java, bind this element using

@Id("layout")
private SplitLayout layout;

The layout orientation will be set to the default horizontal layout (overriding the value set in the template).

Currently, the docs state:

It's important to understand that the element's hierarchical structure for the element injected via @id is not populated and not available on the server side (it's not known). It means that nestedDiv field value which is a Div component doesn't have any child on the server side. Attribute values declared on the client side are reflected to the server side as property values or attribute values.

This, however, doesn't explicitly mention that attributes set on the client-side would be overridden when @id is used.

@Artur-
Copy link
Member

Artur- commented Apr 15, 2021

This sounds more like a bug than something that should be documented

@knoobie
Copy link
Contributor

knoobie commented Apr 15, 2021

@Artur- if I'm not mistaken the behavior, that it should work, was implemented in #8957?

@denis-anisimov
Copy link
Contributor

This is not a bug and it's not fixed by #8957.

The behavior is a consequence of SplitLayout implementation.
The SplitLayout constructor sets the orientation explicitly : setOrientation(Orientation.HORIZONTAL);
(or may be this can be considered as a bug in SplitLayout component).

The value vertical from the example will be set at the time when @Id mapping is established.
Then it's overridden by the server side.
So it works as expected.

I will extend @Id javadocs as a fix for this ticket but please feel free to make a ticket against SplitLayout

@denis-anisimov denis-anisimov self-assigned this Apr 26, 2021
OLD Vaadin Flow bugs & maintenance (Vaadin 10+) automation moved this from WIP to Closed Apr 26, 2021
vaadin-bot added a commit that referenced this issue Apr 26, 2021
vaadin-bot added a commit that referenced this issue Apr 26, 2021
vaadin-bot added a commit that referenced this issue Apr 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

5 participants