backdrop-issues
backdrop-issues copied to clipboard
backdrop
[A11Y][UX] [DX] Replace the more/less toggle links with "details" HTML elements
We currently use these more/less toggles in:
- the modules listing page at
admin/modules(to show/hide dependency info):
- the site status report (to show/hide additional info for errors/warnings):
- the "System updates" page, when there are manual updates (to be introduced with #5085):
These currently are implemented as 90% similar, yet separate pieces of JS, spread across the files of the respective modules providing these admin pages. We should consider using <details> for these, plus some theme_more_less_toggle() function perhaps. That would make the code reusable, consistent, and allow it to be updated in a single place. We'd also inherit any usability/accessibility improvements of the standard HTML element.
For older browsers, we'd need to use a polyfill. Drupal 8+ core currently uses 2 polyfills for this:
- collapse.js is a polyfill for the open/close behaviour from HTML5.1+ recommendation.
- details-aria.js is a polyfill for the HTML-AAM recommendation.
I'd vote for the usage of <details>, but it would look slightly different. So I'm not 100% sure, details elements could fully replace all more/less toggle links.
...but it would look slightly different. So I'm not 100% sure, details elements could fully replace all more/less toggle links.
Here's a PoC PR, which includes #324: https://github.com/backdrop/backdrop/pull/3629
It's rough and needs polishing, but it works as expected and I'd love to get feedback before I work further on it.
There's many things to decide, but most of them belong in #324 - not here. The main things we need to decide here is do we need the arrows that exist for the more/less toggles in the modules listing page, or not (as in the site status report).
PS: if #5085 gets merged, I'll update the PR to convert the more/less toggle in the updates page into a <details> element as well.
Here's a PoC PR, which includes #324: backdrop/backdrop#3629
Why combining the core form item and the toggle PR? Doesn't that make it more likely, that someone kills this issue, too? :stuck_out_tongue_winking_eye:
Anyway, I had a quick look in the sandbox - hey, that replacement might actually work! I have to admit, I only checked the status page and needed a moment to realize, that the more/less thingies are now details elements.
Why combining the core form item and the toggle PR?
Because this is a PoC 😉 ...if people agree that this is a nice thing to have, then I'll work on #324 first, and then once that's in I'll return here to remove any bits of code that aren't needed (basically the '#type' => 'details' Form API implementation and the theme_details() implementation + form_process_details() and form_pre_render_details() functions).
...that replacement might actually work! I have to admit, I only checked the status page and needed a moment to realize, that the more/less thingies are now details elements.
Glad you like it 🙂 ...I'm doing some CSS trickery to prevent clicks on the entire details <summary> and only allow the more/less bit (added via data-* attributes, which makes it work without JS, and is also translatable). That makes these <details> elements behave the same way as those more/less links used to 😉
It was fun working on this PR actually!
I can't believe how seamless the change is.
The code looks great too, but I'm adding the needs-work label for the polyfil :) I found https://www.npmjs.com/package/details-polyfill which has a MIT license.
The code looks great too, but I'm adding the needs-work label for the polyfil :)
FTR: the details element has good support in browsers, except for IE and Opera Mini: https://caniuse.com/details
Here's the link to the polyfill repo on GitHub: https://github.com/rstacruz/details-polyfill
We are triaging issues today to add the [AllY] tag for an upcoming Open Source Day sprint on accessibility issues. There is a suggestion that this change would bring with it accessibility improvements, so I'm adding the tag here.
I think that this has a good chance of getting in for 1.20. I plan to work on adding the polyfill over the next week/weekend.
I just carefully re-read this thread, and noticed this comment I made back in May:
this is a PoC 😉 ...if people agree that this is a nice thing to have, then I'll work on #324 first, and then once that's in I'll return here to remove any bits of code that aren't needed...
So basically this is blocked by (or depends on) #324.
This is not blocked any more, so I can resume work on it. Hoping to do that soon.
Consider also using this for the placeholder examples help text on the Layout configuration page admin/structure/layouts/manage/*/configure. See https://github.com/backdrop/backdrop-issues/issues/5399#issuecomment-1002820944 and preceding discussion.
This is ready for review again. Custom more/less toggles replaced with details element in the following places:
- the module dependencies/details in
admin/modules - the placeholder examples in
admin/structure/layouts/manage/%/configure - the status page in
admin/reports/status
@klonos Are the arrow symbols new? At first sight, they look a bit prominent to me, especially on the admin/reports/status page which is already a quite noisy.
Another question: the details text on admin/structure/layouts/manage/home/configure is show examples / hide examples, i.e. in lower case. Before, it was upper case; why the change? Lower case looks strange with more than one word, in my opinion.
Are the arrow symbols new?
Right, I forgot about that 🤦🏼 ...so the more/less links in status page were using no icons, but the ones in the modules listing were using custom ones (crafted via CSS). Since we now have a consistent way to create these, and since they will be added in other places as well (layout placeholder examples, system updates etc.), we need to decide what we'd use. Options are:
- no icons at all (the previous PR was doing that). I like this approach compared to the previous CSS solution (see next point).
- CSS-crafted icons: I personally don't like these, as I find it to be "too much code" for something as simple as adding an icon. This is an "obscure" solution, that is hard to understand and customize. On top of that, I don't know how to implement them in a clean way now that we are using
content: ' ' attr(data-close-label)insummary::afterin CSS (to display open/close text that can be customized/translated). I've tried a couple of things, but I could only get either just text, or just CSS arrows to show - not both at the same time. We could try to add<span>s etc. but it wouldn't be a very "clean" implementation. If we choose to go back to this option, help with implementation is appreciated. - we could use chevron .png icons. For example, we could:
- convert the .svg icons used in the Claro theme from https://www.drupal.org/project/drupal/issues/3057772
- create our own .png icons from fontawesome:
- use something as simple as +/- symbols (text) that can easily/cleanly be added to the
content:CSS property. - use unicode arrow characters: this can easily/cleanly be added to the
content:CSS property, and provides a straight-forward way to customize the icons in custom/contrib admin themes. We have a wide range of options to choose from. I went with a random choice in the current PR, but I agree that the arrows I've chosen originally were a bit prominent. We could be more subtle - I've updated the PR to use these instead:
Please play around with alternatives, and let me know which would be best to use. There's many resources on the internet that can help, like arrow up / arrow down.
...
show examples/hide examples...was upper case; why the change? Lower case looks strange with more than one word, in my opinion.
That was not intentional, and I have no preference I think. I've updated the PR to switch it back to title case.
Let me start by saying I really like the notion of regularizing how we do these expandable more/less things, and I think there's lots more places that would benefit from using them, so having a regular pattern to follow will be great!
That said, though, I think we're not quite there with this one for all use cases. On the module page (illustrated in the immediately preceding comment), it looks pretty good. On admin/structure/layouts/manage/home/configure, though, it looks like this (arrows highlighted):
Years of expanding fieldsets have taught me that when I see a right-pointing triangle at the left of a bit of text, I can click it and expand the text (or just click on the text itself), as, for example, on the Layouts listing page:
So I don't think we should show that right-pointing triangle if it's not going to be clickable. Or, perhaps better, can we make it clickable?
And then, related to that: that triangle is nicely bold, and visually is much stronger than the thin little up/down arrow at the far right. It feels like since that clickable thing potentially makes a significant change in the page, it warrants being a little beefier. Even the original up/down triangles were a bit small. Current PR:
Previous:
Another nice thing about the previous version is that the same isoceles triangle is used for both fieldsets and more/less toggles and dropbuttons. It would be nice to stay consistent with details elements and use the same shape triangle everywhere there is an expanding chunk of stuff:
So I don't think we should show that right-pointing triangle if it's not going to be clickable.
That default arrow from the <details> element is not meant to be shown - thats what the CSS I've added is meant to be doing, but I'm assuming that this must be a browser compatibility thing. In Chrome/Brave, it looks like this for me (no arrow on the left):
What browser are you testing on @bugfolder ? ...have you cleared caches?
...that triangle is nicely bold, and visually is much stronger than the thin little up/down arrow at the far right. ... it warrants being a little beefier.
I had it like so before:
...but then I changed it because of the feedback provided by @olafgrabienski ...we need to decide on an icon that works well for all 😅 ...that's why I said:
Please play around with alternatives, and let me know which would be best to use.
Another nice thing about the previous version is that the same isoceles triangle is used for both fieldsets and more/less toggles and dropbuttons. It would be nice to stay consistent with details elements and use the same shape triangle everywhere there is an expanding chunk of stuff:
Right. I guess we might go back to that triangle I don't like 😅 (I wish there was a unicode character for it)
I wish there was a unicode character for it
There is!:
- https://unicode-table.com/en/25B2
- https://unicode-table.com/en/25BC
What browser are you testing on @bugfolder ? ...have you cleared caches?
Safari on Mac, and yes, cache is cleared (also, I turned off CSS aggregation on the sandbox so I could see where stuff was coming from).
Here's the HTML details from Safari:
Looks like that triangle is the "-webkit-details-marker".
Safari on Mac, and yes, cache is cleared...
Thanks 👍🏼 ...I'm on a Mac too, but not tested on Safari. Sorry about that. I'll do some cross-browser testing and update the PR accordingly.
...in the past, when we needed to make a decision on icons (like for the admin bar etc.), it helped creating various PRs with different options that people could test. Then we'd post screenshots in separate comments on the issue, and people would vote with 👍🏼 or 👎🏼 ...would that help, or do people find multiple PRs annoying?
I'm happy to respond to either, but for just choosing the icon, posting screenshots of the various contenders would work for me.
PR updated once again:
- icon changed to a triangle that matches the rest of the places we toggle opening/closing of things
- added RTL support (some weirdness about how
::before/::afterCSS selectors behave in LTR vs. RTL)
There's still one minor annoyance remaining in RTL with how the toggle arrow needs to be to the left of the toggle text ...but I need a break.
Feedback is still welcomed, so please test things in the PR sandbox and tell me what you think.
PR now rebased to latest 1.x, to include the changes in the freshly-released v1.21.0 of core 😉
Cool, progress! :tada:
@klonos one question: do we really need a polyfill for IE8?
'browsers' => array('IE' => 'lte IE 8', '!IE' => FALSE),
THB in my own projects I'm beginning to forget about IE11, for sure I don't waste any effort on IE8 and older. :grin:
@indigoxela I've added that back in May, after this was suggested by @jenlampton (see https://github.com/backdrop/backdrop-issues/issues/5090#issuecomment-845480075). Happy to remove 👍🏼
I personally also think that IE8 is "dead". Most of the world has moved to using modern browsers, so this makes sense (and is in accordance with our principles re 80% etc.). Having said that, we have #214 to make an official decision, and last comments I see there have the following sentiment:
@klonos
I know that we've decided that this is a 2.x task, but perhaps we should reconsider(?)
@jenlampton
I think we can safely add new things without IE support now ...
So we just need to make it official, and please say the word, and I'll drop it 😉
Testing in Safari on Mac. There's a couple inconsistencies in visual language that would be nice to address.
On admin/modules/list, the disclosure triangle for fieldset is isoceles right triangle, while for details it's equilateral. Any reason we couldn't/shouldn't use the same triangle for both? And to a user, the disclosure arrow is sometimes on the left of its text, sometimes on the right (as in this example). Should we stick to the left for all?
Clicking on the "more >" adds a gray line between the toggle and the newly revealed text (presumably as a result of focus). It goes away if I click somewhere else. I don't think it should appear at all.
On admin/structure/layouts, the "How layouts work" looks exactly like a fieldset, but it has an equilateral triangle disclosure element. And it gets a gray outline box when toggled, unlike fieldsets.
And on admin/structure/layouts/manage/home/configure, there's also the gray underline that appears upon clicking the toggle:
