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

Rename the current Label component to NativeLabel and create a new Label that is compatible with FW8 Label #4384

Closed
gilberto-torrezan opened this issue Jul 11, 2018 · 10 comments · Fixed by #16708

Comments

@gilberto-torrezan
Copy link
Contributor

gilberto-torrezan commented Jul 11, 2018

Currently the Label component creates a <label> element in the client, which makes it not suitable to rendering loose text in the page, since the <label> in meant to be used in conjunction with other elements (usually <input>). This makes it semantically different from the old Label in FW8, which creates a <div> instead.

On top of that, the <label> element is an interactive content element, which makes it behave differently than a regular text in the page. For example, when used inside a Grid cell, clicking on a label doesn't trigger the selection of the row (see #4313 for details).

My proposal to this issue is the following:

  1. Rename the current class from Label to NativeLabel (using the same naming structure as the NativeButton);
  2. Create a new Label class that behaves exactly like the old Label from FW8;
  3. Update demos and starters to use the new Label class;
  4. Update the documentation explaining the difference between those two classes.
@heruan
Copy link
Member

heruan commented Jul 11, 2018

I wouldn't rename Label. New developers approaching Vaadin will likely expect Label to be <label>, as Div is <div> and H1 is <h1>, and FW8 users are being made well aware that many things changed in Flow to be more HTML-intuitive.

I'd see more fit just a note in the Label JavaDoc warning about this.

@denis-anisimov
Copy link
Contributor

I think it's too late to rename.

May be we can do another class and add a reference to it everywhere in javadocs with huge warning.

@mhosio
Copy link
Contributor

mhosio commented Jul 11, 2018

Agree with the comments above. Rename would be just unnecessary breaking change. But we should add some javadoc explaining the difference for the ones coming from FW8 as @heruan suggested. Maybe with a see also -link to Text / Span components.

@gilberto-torrezan
Copy link
Contributor Author

I think it's too late to rename.

This proposal is not for Vaadin 10.

@denis-anisimov
Copy link
Contributor

I think it doesn't matter since the next release should be 100% compatible with V10.

@gilberto-torrezan
Copy link
Contributor Author

And it doesn't need to be for the next version.

The point is to avoid cases like #4313 to happen in the future.

@Legioth
Copy link
Member

Legioth commented Aug 7, 2018

Should we then also rename Div to NativeDiv to stay consistent?

@Legioth
Copy link
Member

Legioth commented Aug 7, 2018

Should also be mentioned that the JavaDocs have been clarified in #4383.

@pleku
Copy link
Contributor

pleku commented Aug 7, 2018

I see no reason to do any drastic changes anymore for this - yes it is something that existing (or swing based) users run into, but there has not really been that much questions and uproar on this topic that would make us want to change things anymore.

So I'm quite close to closing this as won't fix.

@pleku pleku closed this as completed Jun 4, 2021
@pleku pleku added this to the Abandoned milestone Jun 4, 2021
@pleku pleku added the wontfix label Jun 4, 2021
mshabarov pushed a commit that referenced this issue May 16, 2023
Adds NativeLabel as direct replacement for the HTML-based Label component / element.

Fixes #4384
Fixes #3532
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 24.1.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants