WIP make dynamic tabs follow ARIA 1.1 practices by patrickhlauke · Pull Request #28927 · twbs/bootstrap

patrickhlauke · 2019-06-18T19:02:45Z

set `tabindex` as well as `aria-selected` on tabs

- initialisation happens very inelegantly - could do with some advice on how to move this to a function (should it be public, private, static?) - keyboard handling works, but seems to get confused when switching too quickly between tabs sometimes - activating tabs currently done via click() - there must be a nicer way to do this...how? fire _activate? which parameters to pass?

patrickhlauke · 2019-06-18T19:05:43Z

WIP only at this stage, as i'd need some help with two particular aspects:

how to properly run the initialisation code on page load (which sanitizes any tablists on the current page, to ensure they have correct tabindex values, and that one item is always the active/selected one)
how to actually call the tab activation from inside the keyboard handler - currently doing naive click() but there must be a better way (particularly since it seems if moving too quickly between tabs the script doesn't fire correctly and you may end up with two tab panels showing at the same time)

@Johann-S or others...any pointers? (and yes, no unit tests yet...they'll come)

- there's no ARIA pattern that would allow dropdowns in tablists; it cannot be expressed accessibly to AT users. it also has serious usability drawbacks. we're already saying in the docs it should not be used ... this goes a step further and removes the handling for it

…ntation" `role="presentation"` is needed as otherwise AT (like NVDA) will get confused/won't be able to "count" the total number of tabs properly

patrickhlauke · 2019-06-18T20:32:54Z

https://deploy-preview-28927--twbs-bootstrap.netlify.com/docs/4.3/components/navs/#javascript-behavior

only one (the selected/active) tab control can be focused using keyboard
cursor keys left/right change tabs
tabs auto-activate (following the "Tabs with automatic activation" https://www.w3.org/TR/wai-aria-practices-1.1/examples/tabs/tabs-1/tabs.html and https://www.w3.org/TR/wai-aria-practices-1.1/#tabpanel / pattern)
aria-orientation="vertical" uses cursor up/down
removed the code for dropdowns inside tabs, as it's not a sensible/supported pattern for ARIA and has issues, so might as well remove it...this is arguably a breaking change (though from previous question to devs, nobody seemed to really use it JavaScript enabled tab interfaces - is the ability to use dropdowns useful? #23035)

Johann-S · 2019-06-19T07:17:36Z

Hi @patrickhlauke,

About your use of .click() for me the main issue is when the page is loaded you do not create any Tab instance, you just add accessibility attributes, you should initialize Tabs with: const tab = new Tab(element) and after that on your keyboard listener you'll be able to do: Data.getData(element, DATA_KEY) to get a Tab instance.
Like that you'll be able to call any public methods of our Tab component.

I hope what I said is clear 😟 if not, do not hesitate to ask me 😃

patrickhlauke · 2019-06-19T07:54:57Z

About your use of .click() for me the main issue is when the page is loaded you do not create any Tab instance, you just add accessibility attributes, you should initialize Tabs with: const tab = new Tab(element) and after that on your keyboard listener you'll be able to do: Data.getData(element, DATA_KEY) to get a Tab instance.
Like that you'll be able to call any public methods of our Tab component.

I hope what I said is clear 😟 if not, do not hesitate to ask me 😃

I'll admit that I have no clue what all our magic instances and scripts and DATA_KEY things etc actually do at this point...I just cobble together bits from other things that seem to work. I'll have another crack at this tonight though to see if I can work it out...

As for the initialisation...does it make sense to say it should be in a separate function, and to call that rather than cramming it all into the event listener definition there? he main thing for my page initialisation is: it needs to run on page load before the user even starts interacting with any tab interfaces, to make sure the correct tabindices and aria-selected are set (so before any keyboard listener is even called). But not quite sure how to achieve that with all the code BS uses (I was trying some basic approaches, but kept getting various weird errors with my naive approaches). Maybe if you have a basic gist of how it could/should look that could give me a clue...

(that's partly the problem for me with these things...give me basic vanilla JS, and I can probably spaghetti code my way to the solution...but with the various abstractions like SelectorEngine etc that we use in BS, I'm mostly lost now...)

Since that logic has now been expunged

Stick with the naive `tabs[index].click()` as it works correctly, and trying to trigger just `show()` of a tab instance resulted in weird side effects... Tighten the outer `makeArrary`/`forEach` loop (assuming when there's an empty array the forEach just bails, so no need to check the length?)

patrickhlauke · 2019-06-19T11:52:37Z

Think at this stage i got to the end of my understanding/knowledge of how to tackle this for now. @XhmikosR or @Johann-S ... not sure if you have time or inclination to point me to the most sensible way to do the initialization (rather than cramming it all directly into the load event listener thing)? Or is that actually ok? (looks ugly to me)

additionally, one aspect that is still broken: for tab panels with a fade, it seems that using keyboard i can switch tabs quicker than the fade actually happening/completing, leading to situations where a new tab is being shown while the one that was in the process of being shown isn't fully shown yet...and you end up with two tab panels showing at the same time. again, any pointer/indication of how this could be tackled would be appreciated... (despite this, everything else seems to work correctly though from manual testing, so don't think it's far off being usable)

and yes, also need to come up with unit tests to check the new keyboard behavior, and to check the initialisation happens properly...

Johann-S · 2019-06-19T12:14:05Z

I don't think it's ugly to initialize our tabs in the load event, we do that for our other components for example Carousel or Scrollspy, so feel free to do it that way.

I don't have an answer for your other issue, but I'll take a look into it when I'll find the time

patrickhlauke · 2019-06-19T12:36:27Z

if you pardon the enormous GIF for a second, here's an illustration of the problem when switching tabs too quickly with keyboard...

For correct keyboard handling (cursor up/down) when interacting with these

very verbose, but fully tests that cursor keys are handled correctly and have the desired final result (though stops short of checking the actual tab panels themselves)

patrickhlauke · 2019-07-13T21:16:57Z

last remaining (non-trivial) work here is to add some sensible handling of disabled tabs. they should be skipped/ignored (e.g. if i have 3 tabs, and the second one is disabled, then when focused on the first one and pressing right cursor key, the third tab should be focused/become active)

fix the borked examples of dynamic tab panels, and remove the now unsupported tabs with dropdowns

* Revert "Fix readonly-plain-text with button addon (#25871)" This reverts commit ff40e00. * Add migration comment

* Initial spike of consolidated form checks * Stub out forms rearrangement - Prepping to drop non-custom file and range inputs - Prepping to merge custom and native checks and radios (with switches) - Prepping to merge custom select with form select - Moving docs arround so forms has it's own area given volume of CSS * Move input group Sass file to forms subdir * Start to split and move the docs around * Simpler imports * Copyediting * delete overview file * Remove commented out code * remove the custom-forms import * rewrite flex-check as form-check, replace all custom properties * Remove old forms doc * stub out new subpage link section * update migration guide * Update nav, forms overview in page nav, and descriptions * fix check bg position * fix margin-top calculation * rename .custom-select to .form-select * Update validation styles for new checks * add some vertical margin, fix inline checks * fix docs examples * better way to do this contents stuff, redo the toc while i'm at it * page restyle for docs while here * un-callout that, edit text * redo padding on toc * fix toc * start to cleanup checks docs * Rewrite Markdown tables into HTML * Redesign tables, redo their docs * Replace Open Iconic icons with custom Bootstrap icons * Redesign the docs navbar, add a subheader, redo the sidebar * Redesign docs homepage a bit * Simplify table style overrides for docs tables * Simplify docs typography for page titles and reading line length * Stub out icons page * Part of sidebar update, remove migration from nav.yml * Move toc CSS to separate partial * Change appearance of overview page * fix sidebar arrow direction * Add footer to docs layout * Update descriptions * Drop the .form-group class for margin utilities * Remove lingering form-group-margin-bottom var * improve footer spacing * add headings to range page * uncomment form range css * Rename .custom-range to .form-range * Drop unused docs var * Uncomment the comment * Remove unused variable * Fix radio image sizing * Reboot update: reset horizontal ul and ol padding * de-dupe IDs * tweak toc styles * nvm, fix dropdown versions stuff * remove sidebar nav toggle for now * broken html * fix more broken html, move css * scss linting * comment out broken helper docs * scope styles * scope styles * Fixes #25540 and fixes #26407 for v5 only * Update sidebar once more * Match new sidenav order * fix syntax error * Rename custom-file to form-file, update paths, update migration docs for previous changes in #28696 * rename back * fix size and alignment * rename that back too

Corrected minor spelling error.

Johann-S · 2019-07-23T12:31:33Z

Hey @patrickhlauke, I'm sure you'll hate me 😆 but I removed QUnit and switch to Jasmine, so now your unit tests should go there: https://github.com/twbs/bootstrap/blob/master/js/src/tab/tab.spec.js

js/tests/unit/tab.js have been removed, so if you need any help do not hesitate 😉

patrickhlauke · 2019-07-23T12:46:22Z

heh, the dangers of taking too long / not having time to get this over the line. no worries, i'll have to look at this jasmine thing eventually anyway, might as well do it as part of this ;)

patrickhlauke · 2020-08-02T12:44:06Z

would love to renew efforts along these lines...at this point, probably worth scrapping this PR and starting from scratch again. but my lack of JS chops will probably still raise its head once I hit the trickier parts. so preemptively asking: anybody (@Johann-S, others?) able to collaborate on a new take? I can explain what things should do, but once it gets to integrating that code and how all the event model in BS works...

patrickhlauke · 2020-08-26T10:27:43Z

putting out a call again for anybody who'd have time/inclination to help take this forward with me and help me overcome my woefully lacking JS skills...

XhmikosR · 2020-11-17T23:38:03Z

@patrickhlauke if you could make this from scratch against main, it should make things easier to look into.

patrickhlauke · 2021-04-11T10:22:29Z

Superseded by #33079

patrickhlauke added 2 commits June 17, 2019 23:45

Add initial tabindex handling for tabs

f4d8551

set `tabindex` as well as `aria-selected` on tabs

patrickhlauke requested a review from a team as a code owner June 18, 2019 19:02

patrickhlauke force-pushed the patrickhlauke-tabs-kbd-automatic-activation branch from 9d89ccf to 7c90656 Compare June 18, 2019 19:03

patrickhlauke added 3 commits June 18, 2019 20:19

Add explicit note about aria-orientation, add all missing role="prese…

78737b2

…ntation" `role="presentation"` is needed as otherwise AT (like NVDA) will get confused/won't be able to "count" the total number of tabs properly

Merge branch 'master' into patrickhlauke-tabs-kbd-automatic-activation

578444b

Johann-S requested changes Jun 19, 2019

View reviewed changes

Comment thread js/src/tab.js Outdated

Comment thread js/src/tab.js Outdated

Comment thread js/src/tab.js Outdated

Comment thread js/src/tab.js Outdated

Comment thread js/src/tab.js Outdated

Initial code tweaks based on comments

53e954c

patrickhlauke force-pushed the patrickhlauke-tabs-kbd-automatic-activation branch from b501c94 to 53e954c Compare June 19, 2019 09:23

patrickhlauke added 3 commits June 19, 2019 10:48

Remove dropdown-in-tabs related unit tests

d722edc

Since that logic has now been expunged

Make note about dropdowns in tabs an actual callout

701977b

Replace for loop with forEach

cbac96d

XhmikosR reviewed Jun 19, 2019

View reviewed changes

Comment thread js/src/tab.js Outdated

patrickhlauke added help wanted js v5 labels Jun 19, 2019

Merge branch 'master' into patrickhlauke-tabs-kbd-automatic-activation

d91293d

Add missing aria-orientation to list group tablists

8de6558

For correct keyboard handling (cursor up/down) when interacting with these

patrickhlauke added the on-hold label Jun 24, 2019

patrickhlauke added 2 commits June 24, 2019 21:14

Merge branch 'master' into patrickhlauke-tabs-kbd-automatic-activation

ef0c5c0

Merge branch 'master' into patrickhlauke-tabs-kbd-automatic-activation

3b64256

Add extensive keyboard handling unit tests

430a120

very verbose, but fully tests that cursor keys are handled correctly and have the desired final result (though stops short of checking the actual tab panels themselves)

patrickhlauke and others added 9 commits July 13, 2019 22:20

Fix visual unit test

a38a3bd

fix the borked examples of dynamic tab panels, and remove the now unsupported tabs with dropdowns

Fixes #28994

76f87e2

Fix typo. (#29008)

349eeeb

separate file for our polyfills to have lighter plugins

e81f20c

Remove attribute selectors (#28988)

bfe5438

Drop support for .form-control-plaintext inside .input-group (#28972)

cc5353d

* Revert "Fix readonly-plain-text with button addon (#25871)" This reverts commit ff40e00. * Add migration comment

dist v5

a13fba8

Update collapse.md (#29025)

4fdae4b

Corrected minor spelling error.

patrickhlauke force-pushed the patrickhlauke-tabs-kbd-automatic-activation branch from df0b910 to 4fdae4b Compare July 13, 2019 21:21

patrickhlauke requested a review from a team as a code owner July 13, 2019 21:21

Merge branch 'master' into patrickhlauke-tabs-kbd-automatic-activation

c11118b

Lausselloic mentioned this pull request Mar 5, 2020

tabs - accessibility issue when using ul/li semantic #30338

Closed

mdo changed the base branch from master to main June 16, 2020 20:18

patrickhlauke marked this pull request as draft June 27, 2020 10:14

patrickhlauke mentioned this pull request Aug 2, 2020

[Accessibility] nav tabs : tabpanel focus issues #31345

Closed

XhmikosR added awaiting-reply has conflicts labels Nov 21, 2020

Update tab.js

897f4db

GeoSot mentioned this pull request Feb 13, 2021

Revamp tabs & follow ARIA 1.1 practices #33079

Merged

1 task

patrickhlauke closed this Apr 11, 2021

patrickhlauke deleted the patrickhlauke-tabs-kbd-automatic-activation branch May 8, 2021 11:58

Uh oh!

Conversation

patrickhlauke commented Jun 18, 2019

Uh oh!

patrickhlauke commented Jun 18, 2019

Uh oh!

patrickhlauke commented Jun 18, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Johann-S commented Jun 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

patrickhlauke commented Jun 19, 2019

Uh oh!

Uh oh!

patrickhlauke commented Jun 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Johann-S commented Jun 19, 2019

Uh oh!

patrickhlauke commented Jun 19, 2019 • edited by XhmikosR Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

patrickhlauke commented Jul 13, 2019

Uh oh!

Johann-S commented Jul 23, 2019

Uh oh!

patrickhlauke commented Jul 23, 2019

Uh oh!

patrickhlauke commented Aug 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

patrickhlauke commented Aug 26, 2020

Uh oh!

XhmikosR commented Nov 17, 2020

Uh oh!

patrickhlauke commented Apr 11, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Johann-S commented Jun 19, 2019 •

edited

Loading

patrickhlauke commented Jun 19, 2019 •

edited

Loading

patrickhlauke commented Jun 19, 2019 •

edited by XhmikosR

Loading

patrickhlauke commented Aug 2, 2020 •

edited

Loading