Bug 8062 - Web Admin input labels aren't clickable
Summary: Web Admin input labels aren't clickable
Status: CLOSED FIXED
Alias: None
Product: ThinLinc
Classification: Unclassified
Component: Web Administration (show other bugs)
Version: trunk
Hardware: PC Unknown
: P2 Normal
Target Milestone: 4.15.0
Assignee: Alexander Zeijlon
URL:
Keywords: frifl_tester, linma_tester, prosaic
Depends on:
Blocks: 3854
  Show dependency treegraph
 
Reported: 2022-12-29 11:16 CET by Pierre Ossman
Modified: 2023-04-26 13:47 CEST (History)
4 users (show)

See Also:
Acceptance Criteria:
* Controls should have labels when the purpose of the control by itself isn't clear. * Clicking a label next to a control should activate the control (check the checkbox, focus the text-input etc.) * Labels should not be focusable using the keyboard. They should not receive a focus-outline and should be skipped in the tab-cycle order. * Where we have disabled controls, the connected label should also appear disabled. * IDs added to elements that labels map to must be unique. * Table cells that are visual labels for input elements should be <th> instead of <td> since we have special styling for these. * HTML labels should map to labelable control elements. * HTML labels should not map to containers that hold multiple control elements.


Attachments

Description Pierre Ossman cendio 2022-12-29 11:16:45 CET
It is expected to be able to click on the label of a checkbox (or radio button) to toggle it, not just the checkbox itself. This doesn't work in tlwebadm.

The technical reason is that we haven't properly tagged up the labels using <label>, so the browser doesn't know the elements are connected.
Comment 1 Samuel Mannehed cendio 2023-01-23 15:46:13 CET
The fact that we haven't properly tagged up labels for many elements in Web Admin also results in that we get the wrong font. We want Poppins for headings and short text-bits (like checkbox labels), and Mulish for longer texts.
Comment 3 Samuel Mannehed cendio 2023-02-20 17:11:53 CET
We had some internal discussions about what method of mapping labels to controls we want to use. There are two options, first:

>    <label>
>      <input type="checkbox">
>      This is a checkbox.
>    <label>

second:

>    <input type="checkbox" id="mycheckbox">
>    <label for="mycheckbox">

The advantage of the first method is that you don't need to assign an ID to each control.
However, the second method is the only one that gives us an opportunity to select labels in CSS. A good example of where that is important is styling for disabled labels (see commit r39743), we want the label of a disabled input to look disabled as well.

The second method is most likely preferable.

Sometimes we want the label before the control. The disable-styling mentioned above will only work if the label comes after the control in the HTML. It is however possible to fix the placement of these labels using CSS.

Note that noVNC has worked around the disabled labels issue using JavaScript:

https://github.com/novnc/noVNC/commit/24584cca897937d21891bf43a983fe679f61dcfd
Comment 4 Samuel Mannehed cendio 2023-02-20 17:41:03 CET
Another option regarding disabled labels is simply to expect the "disabled" property to be set on the labels as well in the HTML.
Comment 9 Samuel Mannehed cendio 2023-02-28 11:05:29 CET
All checkboxes and radio buttons now have clickable labels. Labels are still not clickable for text-inputs and textareas at least.
Comment 13 Alexander Zeijlon cendio 2023-03-16 12:53:09 CET
Clickable labels have now been added for text inputs and textareas. Additionally, non-clickable labels have been added where the label text relates to a set of elements instead of a single element.
Comment 15 Alexander Zeijlon cendio 2023-03-22 13:43:02 CET
Reopening this bug since there are some inconsistencies around labeling inputs in tables.
Comment 19 Alexander Zeijlon cendio 2023-03-22 15:47:06 CET
Labels are now only used such that they map to labelable control elements. See https://developer.mozilla.org/en-US/docs/Web/HTML/Element/label#attributes for more info.

Labels that were previously mapping to a container, holding a set of control elements, have been removed where it does not make sense to map the label to any single one of the elements. An example of this is where we have a label text that visually point to a set of radio options, where clicking the label text should not select a specific option.

This bug is now fixed.
Comment 20 Alexander Zeijlon cendio 2023-03-22 16:21:28 CET
A fix was added to make cursor behavior consistent where we use table headers as visual labels for input elements in a table.

When only a subset of the headers are labeled, we get the effect that the cursor looks like an ordinary mouse pointer when moving over some header texts, while it changes to a text marker when moving over others.

The cursor behavior has now been styled such that the cursor consistently shows a text marker, independent of if the table header text is wrapped in a label or not.
Comment 21 Alexander Zeijlon cendio 2023-03-23 12:31:04 CET
This bug is now fixed.
Comment 22 Linn cendio 2023-03-29 16:25:04 CEST
Removing an acceptance criterion about how to handle labels for a set of control elements, that was contradicting another criterion. The correct way to handle them are:
> * HTML labels should not map to containers that hold multiple control elements.
Comment 23 Linn cendio 2023-03-29 16:30:45 CEST
Tested with Firefox on Fedora 37 with server build 3165. Looked through all Web Admin pages.

> * Controls should have labels when the purpose of the control by itself isn't clear.
Yes, controls like checkboxes, radio buttons and inputs either have a corresponding <label>, or are described in a nearby paragraph.

> * Clicking a label next to a control should activate the control (check the checkbox, focus the text-input etc.)
It does. Looked out for labels for text-input, radio buttons, checkboxes, textareas and selects, both when presented as a table and for other layouts.

> * Labels should not be focusable using the keyboard. They should not receive a focus-outline and should be skipped in the tab-cycle order.
Works, labels are skipped when tabbing.

> * Where we have disabled controls, the connected label should also appear disabled.
Yes, the label text looks grayed out and nothing happens when clicked.

They only place I found this was in the session details popup for an ended session (see bug 8083 comment 23 for tips on how to get an ended user, but instead of being in the popup when closing the session, be on the session list page with the session visible. Click the session when ended, and the disabled element should be visible in the resulting popup).

> * IDs added to elements that labels map to must be unique.
Yes, looked through the code and found all labels to be unique.

> * Table cells that are visual labels for input elements should be <th> instead of <td> since we have special styling for these.
Yes, the visual labels in tables are all styled with <th>.

> * HTML labels should map to labelable control elements.
Yes, did not find mapping to any non-labelable elements. For a list of labelable elements, see: https://html.spec.whatwg.org/multipage/forms.html#category-label

> * HTML labels should not map to containers that hold multiple control elements.
It does not, and for visual labels in tables for e.g. radio buttons, the visual label is not a <label> element.

---
Also tested some additional things:
1) Mouse pointer is correctly shown as text marker for <th> elements.

2) Added label text is clickable, like "(*Required)" when forgetting to confirm deletion or termination

3) Hovering a label shows the usual hover styling on the control element. E.g. hovering the label of a radio button makes it look like the radio button itself is being hovered.

Looked through the code, looks good. Closing
Comment 24 Alexander Zeijlon cendio 2023-04-21 13:40:06 CEST
The Introduction Texts page needs to be modified, we currently use table header elements and labels inconsistently with other pages in tlwebadm.
Comment 28 Alexander Zeijlon cendio 2023-04-21 14:38:33 CEST
On the Introduction Texts page in tlwebadm, the "Language" column in the two tables have been modified:

* The element type of the table cells have been changed from <th> (table header) to <td> since they are not used as table headers.
* The label elements that previously wrapped the text in the table cells have been removed, since the cells do not contain text that is descriptive of the purpose of text inputs in adjacent cells.
Comment 29 Frida Flodin cendio 2023-04-26 13:47:02 CEST
Looks good, checked that comment 28 is indeed fixed and I could not find any issues with it. The code changes look good! Closing

Note You need to log in before you can comment on or make changes to this bug.