Skip to content
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

Fix record tab heading level #3076

Conversation

maccabeelevine
Copy link
Member

From #3039 Accessibility: Moravian Library Report

Pick up location label

Details from https://docs.google.com/document/d/1qII8IRWOk0eP3er-91C5clhTfQq92yPkUbvAtq3xdNU/edit:

LH: Only a heading with location name should be 2nd level, just like the "Similar Units" heading, not 3rd.

@maccabeelevine
Copy link
Member Author

I changed the holdings tab h3 headings to h2s. I agree that's correct since there is no other h2 in between.

In theory I would have said this is wrong though, that the actual tab names should be auto-generated sr-only h2s displayed at the top of each tab's content block. The pattern on the w3.org site is maybe a compromise, it uses aria-labelledby to associate each tab-pane with the text that names the tab in its button. I implemented that for consideration. I did not do any screen reader testing.

@maccabeelevine maccabeelevine marked this pull request as ready for review September 7, 2023 14:22
Copy link
Member

@demiankatz demiankatz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @maccabeelevine! This all makes sense to me, though I'll let the more front-end-facing people weigh in before providing an approval.

The obvious side effect is that headings in tabs are going to be visibly larger. I haven't looked to see how dramatic this is, but is it possible we'll want to add some styling to compensate?

@maccabeelevine
Copy link
Member Author

maccabeelevine commented Sep 7, 2023

The obvious side effect is that headings in tabs are going to be visibly larger. I haven't looked to see how dramatic this is, but is it possible we'll want to add some styling to compensate?

@demiankatz Yeah it's a fair question. But I see there are existing h2 headings in some other tabs, like MARC in the Staff View, or Similar Items in Similar Items, and they have no special styling that I can see. So if it's needed to tweak that we should do so globally i.e. .tab-pane > h2. I'm agnostic whether it's needed.

Edit: the difference at full screen with appears to be 20px to 23px.

@demiankatz
Copy link
Member

Good point, @maccabeelevine, if other tabs are already using h2 then there's a precedent and this just makes things more consistent. I don't have strong feelings about text size in any case, but just wanted to be sure the issue was surfaced. I'm in favor of taking no special action unless somebody complains about the aesthetics. :-)

@demiankatz demiankatz added this to the 9.1 milestone Sep 28, 2023
@demiankatz demiankatz removed the request for review from crhallberg October 3, 2023 10:13
@demiankatz demiankatz merged commit be253ef into vufind-org:dev Oct 3, 2023
6 checks passed
@demiankatz
Copy link
Member

Thanks, @xmorave2!

@maccabeelevine maccabeelevine deleted the accessibility-report-record-tab-headings branch October 3, 2023 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants