Add option to un-sticky table headers to benefit assistive tech users

The Needs Review Queue Bot → tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Sounds like this is largely affecting https://www.w3.org/WAI/WCAG22/Understanding/focus-visible.html

yash.rode → made their first commit to this issue’s fork.

Hi, I was trying to make the patch from #3163765-2: Add option to un-sticky table headers to benefit assistive tech users → compatible with 11.x, but while trying to apply tableheader.js's changes, I noticed that TableHeader is not getting initialised. Can someone tell me how to approach this? For now I have created a new instance of TableHeader which doesn't seem right

Hi, I was trying to make the patch from #3163765-2: Add option to un-sticky table headers to benefit assistive tech users compatible with 11.x, but while trying to apply tableheader.js's changes, I noticed that TableHeader is not getting initialised. Can someone tell me how to approach this? For now I have created a new instance of TableHeader which doesn't seem right

To understand why this might not be working Do a search for sticky-enabled (the class Tableheader looks for to initialize) in the codebase and it's a pretty manageable 15 matches. One of them is this line in Claro that intentionally removes this class from tables.

public static function tablePositionSticky(array $element) {
    if (isset($element['#attributes']['class']) && is_array($element['#attributes']['class']) && in_array('sticky-enabled', $element['#attributes']['class'], TRUE)) {
      unset($element['#attributes']['class'][array_search('sticky-enabled', $element['#attributes']['class'])]);
      $element['#attributes']['class'][] = 'position-sticky';
    }
    return $element;
  }

A git blame will show this was added in the following issue: 📌 Use position: sticky for views sticky table header Fixed which updates Claro to use CSS for positioning sticky.

Another means of narrowing this down would have been to use your browser dev tools to inspect the sticky element and see what is making it sticky.

Either way, it looks like the approach will need to be adjusted to account for changes to core in the 4+ years since the last patch was added.

So for non-Claro themes, Tableheader will need to be updated to make sticky toggle-able. Claro is using a different approach to making the header sticky, and it will require a different toggle implementation based on that.

I tried keeping/removing the `position-sticky` class from the table, which makes it sticky/non-sticky respectively. I will try to implement that solution.

Hi, the current version of the MR works for all other themes, except Claro as discussed in #3163765-19: Add option to un-sticky table headers to benefit assistive tech users → . I have the version of tableheader.js which is working for Claro also, but there are few things that needs to sanitised in that, will get to that tomorrow. Posting that as a patch for now.

This is the working patch for Claro which encorporates suggestions from #3163765-7: Add option to un-sticky table headers to benefit assistive tech users → ,
but unfortunately 📌 Make drupal.tableheader only use CSS for sticky table headers Needs review just got in, which makes all this of no use.

Re-created the tableheader.js for making the sticky optional. Added the sticky header checkbox before the sticky header.
need to be cleaned and use local storage to handle the page refresh.

Patch testing will be deactivated in 32 days → so it's best to do all of this in Gitlab.

1. +++ b/core/core.libraries.yml
   @@ -680,6 +680,14 @@ drupal.tabledrag.ajax:
   +      - core/jquery
   +      - core/drupal
   +      - core/drupalSettings
   +      - core/once
   +      - core/drupal.displace
   

   Currently none of these dependencies are used, but core/drupal probably should be used so Drupal behaviors are used instead of DomContentLoaded

2. +++ b/core/misc/tableheader.js
   @@ -0,0 +1,32 @@
   +              Sticky Header
   

   Not translated

3. +++ b/core/misc/tableheader.js
   @@ -0,0 +1,32 @@
   +document.addEventListener('DOMContentLoaded', function () {
   

   Was this a GPT suggestion? GPT loves DOMContent loaded 🙂. Drupal behaviors would be used for something like this

4. +++ b/core/misc/tableheader.js
   @@ -0,0 +1,32 @@
   +  const stickyHeaderElement = document.getElementsByClassName('views-table')[0];
   

   Since you're just getting the first match document.querySelector() is cleaner

5. +++ b/core/misc/tableheader.js
   @@ -0,0 +1,32 @@
   +    const checkbox = document.querySelector('[data-drupal-toggle-sticky-header]');
   

   The theme function might be overridden and not have this selector. While it is ultimately up to the developer to make sure these things are in place, you have the return value 2 lines above and could reference whatever is returned by stickyHeaderCheckbox

6. +++ b/core/misc/tableheader.js
   @@ -0,0 +1,32 @@
   +    checkbox.addEventListener('change', function () {
   

   Drupal uses arrow syntax

7. +++ b/core/misc/tableheader.js
   @@ -0,0 +1,32 @@
   +      if (checkbox.checked) {
   +        if (!stickyHeaderElement.classList.contains('sticky-header')) {
   +          stickyHeaderElement.classList.add('sticky-header');
   +        }
   +      } else {
   +         if (stickyHeaderElement.classList.contains('sticky-header')) {
   +           stickyHeaderElement.classList.remove('sticky-header');
   +         }
   +      }
   

   All this logic can be just one line by using toggle()

I am working on adding the test.

This is consuming quite a bit of space for a control that most users will be indifferent to.

The location "show row weights" button, while not perfect (as mentioned in #7) is probably a better option as it is less greedy while still being visible.

I have selected demo_umami profile because we get the ready made list of contents to check the sticky feature? Should I remove this and add content manually?

Yep, you'll need to add some dummy content, the need for nodes to fill the table isn't justification to use a whole profile. Creating what you need can be done in a handful of lines with a loop that calls $this->drupalCreateNode() enough times to fill the view.

It is not working for stark theme, I will have to debug that more to figure that out.

So tested out on the permissions page and not sure how I feel about it haha. I like the control it gives but also wonder if it should be a theme setting. "Allow users to toggle sticky headers" so if you don't really want it you could turn off.

Functionally though working as advertised.

Leaving in NR for additional feedback

+1 for theme setting.

smustgrave → changed the visibility of the branch 3163765-add-option-to to hidden.

Cleaning up MR and patches some. Could we add the theme setting?

It is mainly for the accessibility users, so it makes more sense for it to be just above the table instead of going to theme settings page and changing it there.

Then I may be a -1 for it as it feel kinda awkward with the current placement

I'm not sure a theme setting would work well here for a few reasons:

• This doesn't work as well as a site-level setting. It's something that should be available on a per-user level
• An AT-using user that wants to disable sticky should not need to have permissions to change theme settings
• There's a reduced likelihood of a user knowing such an option even exists as it is no longer in the same location as the UI element it impacts

I mentioned the hide/show weights toggle in #28 as a table-specific A11y setting that has existed in Drupal core for some time without much objection. Were the positioning of the sticky toggle more similar to hide/show weights I suspect the awkwardness would be mitigated.

Were the positioning of the sticky toggle more similar to hide/show weights I suspect the awkwardness would be mitigated.

I'd agree with that statement too.

Addressed feedbacks mentioned in #38.

The current MR seems to add the checkbox twice on admin/content - once before and once after the toggle for in a narrow view toggling on and off the low priority column. The order of the two toggles is something to be decided. The toggle seems to be missing admin/content/blocks, admin/content/files or admin/content/media - the last of which also has an alternative grid-view that doesn't need one.

On permissions page the checkbox is also doubled, but there are no other controls in between.

I think we should have do both the toggles with similar implementation. Now one is a checkbox and the other is a button. Styling can then be decided per theme if one would like to use a pin-icon for sighted users etc.