- Issue created by @hexabinaer
- 🇩🇪Germany tobiasb Berlin
For the record:
* size comes core datetime element, should only be added, when type is text
* same for placeholder
* funny, but cleverdata-help
comes from core just for IE11 https://www.drupal.org/node/3081864 →
(Todo find/create core issue) - 🇩🇪Germany hexabinaer Berlin, Germany
Oh, I wasn't aware of that. Thanks for the research!
- 🇳🇱Netherlands johnv
Thanks for all your effort.
Some questions about the MR:
-1,7 : why remove the function name?
-95,12, 126,9: why check for slot, nextSlot, if thisis not done in the upper lines, and also not in 198 (copy)?
-198, -218: plese explain why 'data-drupal-selector$' is replaced by 'data-action' ? (mind the wild card) , since thisis introduced only recently.
-198: $(this).parents('.dropbutton-wrapper') : I guess you tested with the dropbutton issue implemented?in OfficeHoursBaseSlot , you replace the links with 'button'. Not sure from the comments above, is this preferable to the dropbutton? Is thisin line with the drupal core standard behaviour?
- 🇳🇱Netherlands johnv
Just for future reference, please add inline comments like we did in the past: "// Add a label/header/title for accessibility (a11y) screen readers."
- 🇩🇪Germany hexabinaer Berlin, Germany
in OfficeHoursBaseSlot , you replace the links with 'button'. Not sure from the comments above, is this preferable to the dropbutton? is this preferable to the dropbutton? Is thisin line with the drupal
@johnv this is one of the major improvements for screen reader users: buttons trigger actions/behaviour whereas links should take you to a new page. It's about semantics, not about what the element looks like. That said, a dropbutton ist just a wrapper (that must be accessible, too, of course) to save space in complex UIs. We know the concept from Views (legitimately wrapping operations links as well as from the optional Paragraphs dropbutton widget (wrapping buttons).
It is an ongoing process to replace inappropriate elements, in core as well as in contrib.
- 🇩🇪Germany tobiasb Berlin
@johnv It is better to write your questions in the MR.
* IIFEs does not have a function name. example: https://git.drupalcode.org/project/drupal/-/blob/11.x/core/misc/ajax.js?...
* The check for slot is gone
* I reuse the pattern data-drupal-selector again
Drupal core does not support native buttons via form api, therefore we hardcode the buttons. But I added a todo with a link to 📌 Use form element type instead of Needs work .
- 🇩🇪Germany hexabinaer Berlin, Germany
@johnv re.
I guess you tested with the dropbutton issue implemented?
Not exactly but we took the understandable user story into account. Buttons would of course take up even more space than the links. tobiasb took another turn on the code, added comments (he says sorry for adding in late) and did some refactoring.
A follow-up idea would be to give site-builders the option to decide whether they want to wrap the options in a dropbutton or not.
Further answers to your questions (or new ones): let's use code comment threads to make sure we're still referencing to the same lines of code.
- 🇳🇱Netherlands johnv
Above patch adds the Announcement and some coding standards.
I also added'#wrapper_attributes' => ['header']
to Week and List, next to ExceptionsThe attached file contains the remainder of the patch. Sorry for not using fancy tools.
For the time element, need to check that separately - perhaps in a separate ticket?
For the dropbutton, I'd like to combine with aforementioned ✨ Replace operation links in the widget with a dropbutton Needs work . I will update that ticket.
For "Action links have an empty href attribute, no aria attributes", I see no other way atm? - 🇩🇪Germany hexabinaer Berlin, Germany
Looking into it soon.
Just to let you know, I noticed one more: The season titles should rather be rendered in
<caption>
markup. Replacing the weekday table header cell with it might be confusing (arguably). It would be helpful for theming, too because season names might consume more space that the weekdays (especially when using 2ch or 3ch abbreviations).I'll get back to that.
- Assigned to tobiasb
- Status changed to Needs work
11 days ago 7:50am 23 December 2024 - 🇳🇱Netherlands johnv
@hexabinaer, at least in the 'table' formatter, to me it seems the season title is already in the caption.
- 🇳🇱Netherlands johnv
Let's reopen this issue, or open a new one, when we find something to improve.