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 support for @Id without value #3758

Merged
merged 2 commits into from
Mar 26, 2018

Conversation

gilberto-torrezan
Copy link
Contributor

@gilberto-torrezan gilberto-torrezan commented Mar 23, 2018

When @Id is used without value, the name of the field will be used to map the element on the template to the field on the Java class.


This change is Reviewable

When @id is used without value, the name of the field will be used to
map the element on the template to the field on the Java class.
@gilberto-torrezan gilberto-torrezan self-assigned this Mar 23, 2018
@denis-anisimov
Copy link
Contributor

Reviewed 3 of 3 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


flow-server/src/main/java/com/vaadin/flow/component/polymertemplate/TemplateDataAnalyzer.java, line 225 at r1 (raw file):

throw new IllegalStateException(String.format(

Quoted 5 lines of code… > "Class '%s' whose template URI is '%s' contains field '%s' annotated with @id%s. " > + "Corresponding element was found in a sub template, " > + "for which injection is not supported.", > templateClass.getName(), htmlImportUri, field.getName(), > emptyValue ? "" : "(\"" + id + "\")"));

The message doesn't mention that element is referenced by id (in case it's derived from the field name).
I think the message should be extended with this information when id is the field name.


Comments from Reviewable

@gilberto-torrezan
Copy link
Contributor Author

Review status: 2 of 3 files reviewed at latest revision, 1 unresolved discussion.


flow-server/src/main/java/com/vaadin/flow/component/polymertemplate/TemplateDataAnalyzer.java, line 225 at r1 (raw file):

Previously, denis-anisimov (Denis) wrote…

throw new IllegalStateException(String.format(
"Class '%s' whose template URI is '%s' contains field '%s' annotated with @id%s. "
+ "Corresponding element was found in a sub template, "
+ "for which injection is not supported.",
templateClass.getName(), htmlImportUri, field.getName(),
emptyValue ? "" : "("" + id + "")"));

The message doesn't mention that element is referenced by id (in case it's derived from the field name).
I think the message should be extended with this information when id is the field name.

Done.


Comments from Reviewable

@denis-anisimov
Copy link
Contributor

:lgtm:


Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@gilberto-torrezan gilberto-torrezan merged commit 84df017 into master Mar 26, 2018
@gilberto-torrezan gilberto-torrezan deleted the gilberto/at_id_without_value branch March 26, 2018 08:31
@denis-anisimov denis-anisimov added this to the 1.0.0.beta5 milestone Mar 29, 2018
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

2 participants