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

Warn users about incorrect usage of Label #3532

Closed
marcushellberg opened this issue Feb 9, 2018 · 16 comments · Fixed by #16708
Closed

Warn users about incorrect usage of Label #3532

marcushellberg opened this issue Feb 9, 2018 · 16 comments · Fixed by #16708

Comments

@marcushellberg
Copy link
Member

marcushellberg commented Feb 9, 2018

When I incorrectly use Label in V10, I want to get a warning message.

In Vaadin 8 and earlier Label was used for text content. In V10, it maps to <label>. This will likely lead to a lot of incorrect markup that's possibly confusing for screen readers.

Suggestion: Log a warning in the console if a Label is used without for or a child element.

@Legioth
Copy link
Member

Legioth commented Feb 12, 2018

Is this really a problem in practice? How are screen readers interpreting <label> elements without for or children?

@pleku
Copy link
Contributor

pleku commented Feb 12, 2018

Warning message in console on client or server ?

@marcushellberg
Copy link
Member Author

On the server. I don't think that developer using the Java API will look at the browser console so actively.

I don't know that screen readers will have problems with the incorrect usage, but I would guess they would try to use <label> info.

It may also lead to layout confusion as <label> is an inline block as opposed to block/inline-block that Label had.

@Legioth
Copy link
Member

Legioth commented Feb 13, 2018

So the first step would be to investigate whether this is actually a problem at all?

@marcushellberg
Copy link
Member Author

Well, it does lead to incorrect markup at least.

@jreznot
Copy link
Contributor

jreznot commented Feb 13, 2018

Guys from business / enterprise development usually consider Label as ANY "textual message on screen". From my FW experience I would say that caption is associated with Label for UI field, but Flow has much more common with HTML5. That is the problem with terminology.

Wnt added a commit to Wnt/flow-and-components-documentation that referenced this issue Apr 5, 2018
denis-anisimov pushed a commit to vaadin/flow-and-components-documentation that referenced this issue Apr 5, 2018
* Do not use Label when not labeling input fields

Label should only be used when labeling input fields.
See
vaadin/flow#3532
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/label

* Do not use Label when not labeling input fields
@pleku pleku added this to the Candidates milestone Oct 8, 2018
@knoobie
Copy link
Contributor

knoobie commented Jan 15, 2021

@marcushellberg instead of logging a warning, I would propose to rename the current Label to NativeLabel and create a "new" Label that just extends Text.

Of course this is a breaking change, but it's worth for the end user and probably for the developer coming from other frameworks like swing.

@pleku what do you think?

Edit: My gut feeling, that this could be a major problem, is manifesting. It did not take one hour to see code that misuses label for displaying text in the community discord.

About Leif's screen reader point:

It's not a problem per se for AT, but it's a WCAG violation https://www.w3.org/TR/WCAG20-TECHS/H44.html

Edit 2: Okay, found the corresponding issue and some thoughts against renaming. #4384

@knoobie
Copy link
Contributor

knoobie commented Apr 4, 2023

This is still an issue and people still abuse the Label making their application less accessible. I'm open to provide an PR for my suggestion from above (adding NativeLabel and deprecating of Label / removal in 25) if you wanna tackle this.

cc: @rolfsmeds @mshabarov

@rolfsmeds
Copy link

I would not be against the rename, but it would presumably have to wait until the next major.

A warning on the other hand could be added in a patch release even.

@Legioth
Copy link
Member

Legioth commented Apr 12, 2023

It's not enough to only add a warning since there are also legitimate use cases for the functionality. One possibility would be to deprecate Label and add NativeLabel as non-deprecated.

@knoobie
Copy link
Contributor

knoobie commented Apr 12, 2023

Yeah, that's what I wanna do if you guys are on board.

Step 1: Add a warning in onDetach() if the Labelhas no forattribute set (Using detach instead of attach to make sure that it works reliable even if people add the input field later)
Step 2: Add NativeLabel and deprecate Label
Step 3: Remove Label

@rolfsmeds
Copy link

How about also deprecating Text and introducing TextNode as a non-deprecated alias, so that we can later introduce a Text component to serve as a generic text element replacing V8 Label?

@knoobie
Copy link
Contributor

knoobie commented Apr 12, 2023

I really like that, even tho I probably have to refactor / copy paste some of my code because I'm heavily relying on Text currently. Worth another issue I would say!

@rolfsmeds
Copy link

There is one and I'll link it here as soon as I find it amongst the hundreds of other issues containing the term "text".

@knoobie
Copy link
Contributor

knoobie commented Apr 27, 2023

Created a PR to push the discussion forward. Deprecating and later removal of Label could also help the modernization team and the design system team.

The design system could add their own "Label" component that has way more and better feature than the regular label and could be used more widely and replace most of the direct need of creating spans or divs to show text (looking a Rolfs idea about a styled text component)

In addition, the modernization team could more easily migrate from the old to the new label / swing users and old Vaadin users already know this term.

For developers this would (hopefully) get them a new and better label component in the long run and should drastically reduce accessibility problems people have created over the last years when building applications with this component in a wrong way. (Because of a history with V8, a swing history or just because they didn't know better and used it because they can)

Creating html labels should not be needed in normal use cases and the migration path to the new native label is literally a simple replacement that can be done by a single IDE replacement call.

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