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

Tablist Examples: Improve support for magnification #2625

Merged
merged 5 commits into from Apr 10, 2023
Merged

Conversation

jongund
Copy link
Contributor

@jongund jongund commented Feb 23, 2023

Resolves #2284 and #2257 with the following changes:

  1. Changes CSS so it defines tab width as a percentage of screen width. This prevents screen magnification from causing horizontal scrolling. As the screen is magnified ,the height of the tabs increases and the tab content reflows to fit the new dimensions.
  2. Adds explanation of the use of percentage to the Accessibility Features section.

Preview Updated Tablist Examples

Review checklist


WAI Preview Link (Last built on Mon, 10 Apr 2023 06:55:59 GMT).

@jongund
Copy link
Contributor Author

jongund commented Mar 2, 2023

@mcking65
There are many failing test in unchanged example files, the error message always seems to be "Timed out while waiting for WebDriver server".
Not sure what to do to fix this regression testing problem.

@jongund
Copy link
Contributor Author

jongund commented Mar 2, 2023

@mcking65
Same problem with the updates to the Listbox examples, failing regression tests of unchanged example files.
Seems to be the same error message: 'Timed out while waiting for WebDriver server"

@howard-e
Copy link
Contributor

howard-e commented Mar 2, 2023

There are many failing test in unchanged example files, the error message always seems to be "Timed out while waiting for WebDriver server". Not sure what to do to fix this regression testing problem.

@jongund @mcking65 tracking in #2636 and filed a temporary fix in #2640 which is pinning the node version back to v16 when running the regression tests in CI.

@mcking65
Copy link
Contributor

mcking65 commented Mar 7, 2023

@jongund

Does #2257 fit within the scope of this PR? If it would expand scope significantly, let's not include a fix for it, but if it is related to the code you are changing and simple, it might be a good issue to resolve at the same time.

@a11ydoer a11ydoer added the agenda Include in upcoming Authoring Practices Task Force meeting label Mar 7, 2023
@jongund
Copy link
Contributor Author

jongund commented Mar 20, 2023

@mcking65
I believe the high contrast issue is also fixed for the button elements used as tabs, I will check it out before tomorrows meeting.

@jongund
Copy link
Contributor Author

jongund commented Mar 25, 2023

@mcking65
I checked out the updated tablist example in high contrast and it passes our high contrast tests. So the high contrast issue can also be closed with this pull request.

@css-meeting-bot
Copy link
Member

The ARIA Authoring Practices (APG) Task Force just discussed PR 2625 - Tablist Examples: Improve support for magnification by jongund.

The full IRC log of that discussion <jugglinmike> Subtopic: PR 2625 - Tablist Examples: Improve support for magnification by jongund
<jugglinmike> github: https://github.com//pull/2625
<jugglinmike> Matt_King: jongund isn't here today, but I'd like to assign some folks to review, if possible
<jugglinmike> Andrea: I can help out!
<jugglinmike> Andrea: I can do visual review and accessibility review
<jugglinmike> Matt_King: This is a really narrowly-scoped set of changes to CSS
<jugglinmike> Matt_King: I don't know if there's much of a code review to be done here
<jugglinmike> Alex_Flenniken: I can do the code review
<jugglinmike> (Matt_King updates the review checklist)
<jugglinmike> (Matt_King adds Andrea's account to the GitHub Team named "aria contributors" in order to authorize review)
<jugglinmike> Matt_King: I think that's all we can do for pull requests without jongund present

Copy link
Contributor

@alflennik alflennik left a comment

Choose a reason for hiding this comment

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

The code looks good, and I confirmed that it works by comparing the current version with the updated version - for both examples I verified that the current version is causing horizontal scrollbars and that the updated version fixes the issue.

Copy link
Contributor

@mcking65 mcking65 left a comment

Choose a reason for hiding this comment

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

Editorial review complete; looks good.

@css-meeting-bot
Copy link
Member

The ARIA Authoring Practices (APG) Task Force just discussed PR 2625 - Tablist Examples: Improve support for magnification by jongund.

The full IRC log of that discussion <jugglinmike> Subtopic: PR 2625 - Tablist Examples: Improve support for magnification by jongund
<jugglinmike> github: https://github.com//pull/2625
<jugglinmike> Andrea_: I think this just requires a final accessibility review from me
<jugglinmike> Andrea_: I have to set up some ATs, but I expect to finish that today
<jugglinmike> Matt_King: Looks like you have some suggested edits in here
<jugglinmike> Andrea_: Yeah, a couple of typos
<jugglinmike> Matt_King: looks like Alex_Flenniken is also assigned to this, too
<jugglinmike> Matt_King: We may be able to include this for April 11, too. That'd be great

@andreancardona
Copy link
Contributor

andreancardona commented Apr 5, 2023

Other than a few typos - a11y check has been done and approved!

@andreancardona
Copy link
Contributor

@alflennik are the above changes something I can commit or should someone else? Other than those fixes, I'd like to approve. thank you! :)

@alflennik
Copy link
Contributor

@andreancardona oh good question, I'm not sure, I've seen different kinds of approaches in past APG PRs - I've seen Matt commit editorial changes directly to PRs in the past, but I've also seen the person who opened the PR being responsible for making recommended changes. @jongund I'm curious whether you prefer for Andrea to commit spelling changes directly to the PR or for you to be responsible for that.

"insure" > "ensure"

Co-authored-by: Andrea N. Cardona <cardona.n.andrea@gmail.com>
@mcking65
Copy link
Contributor

@andreancardona wrote:

@alflennik are the above changes something I can commit or should someone else? Other than those fixes, I'd like to approve. thank you! :)

If fixing an obvious typo, feel free to either commit or use the suggestion feature as you did.

If changing wording, please use the suggestion feature.

Thank you very very much for the thorough and detailed review!!

@mcking65 mcking65 merged commit 9a729ec into main Apr 10, 2023
20 checks passed
Tabs Pattern and Examples Development Project automation moved this from In Progress to Complete Apr 10, 2023
@mcking65 mcking65 deleted the bugfix/issue-2284 branch April 10, 2023 06:58
@mcking65 mcking65 added bug Code defects; not for inaccurate prose Example Page Related to a page containing an example implementation of a pattern labels Jun 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agenda Include in upcoming Authoring Practices Task Force meeting bug Code defects; not for inaccurate prose Example Page Related to a page containing an example implementation of a pattern
7 participants