-
-
Notifications
You must be signed in to change notification settings - Fork 32
aria hidden #137
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
aria hidden #137
Conversation
when there is no active tab set we need to remove the attribute aria-hidden. Other wise the tab is invisible to a screen reader PR for joomla/joomla-cms#28122
|
|
||
| // Fallback if no active tab | ||
| if (!this.hasActive) { | ||
| tabsEl[0].setAttribute('active', ''); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assume you've tested this - because right now I'm extra confused on code review about the difference between this line and the line below on 145. As a result not sure if this belongs here or in that querySelector section :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldnt work it out either. did it all by trial and error
from what I could tell when we dont have an active tab defined on load the code here just added "active"
we have code elsewhere that puts ariahidden on everything
It is only removed when you have a real active tab hence this code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW these lines till this.hasActive = true; are a fallback in case the backend throws some markup without any tab active. The lines starting from 142 also are referring to the links and the lines before to the tab content, so it's a sync dance
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it always opens without an active tab unless one has been saved in the session and a tab with that name exists or the url included the #tab fragment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it always opens without an active tab
IIRC there is some code that sets the active attribute in one of the 2 relative layouts. Maybe there's a bug there. Normally the active tab should be set in the backend, the code for the hash or the localsorage should be applied after this part as it's a client related code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont think there can be or this section of the js would never have been needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fallback is there because there should always be a fallback. Also I think that this parts reverts the active tab form the hash, if I read the code correctly (eg no active code from the backend but hash in the url => the hash will not be respected, order of execution)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I already see the bug: JHtmlUiTab that will not compute it needs to be HTMLHepler or better __METHOD__ as few lines above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
__METHOD__ won't work as we need it to check for the startTabSet method - not the addTab method. We'd need to use __CLASS__ i guess
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
joomla/joomla-cms#28179 PR - but I don't think this will fix it - __METHOD__ will still use the JHtmlUiTab class I think as it's not namespaced yet. But it's good practice to use __CLASS__ anyhow as we will need to namespace at some point. If it fixes it all the better
|
great thanks - if you could merge #136 and do you a new release that would be awesome |
when there is no active tab set we need to remove the attribute aria-hidden. Other wise the tab is invisible to a screen reader
PR for joomla/joomla-cms#28122
obviously this will need a new release after merging