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

Infrastructure: update skipto.js to version 5.2.1 #2807

Merged
merged 9 commits into from
Feb 13, 2024
Merged

Conversation

jongund
Copy link
Contributor

@jongund jongund commented Sep 19, 2023

Updated skip.js to use public distribution.
This requires scripts referencing skipto.js to include the following data attribute:

  <script data-skipto="colortheme: aria; displayoption: popup" src="../../shared/js/skipto.js"></script>

This is related to issue 259 in wai-aria-practices repository.


WAI Preview Link (Last built on Tue, 06 Feb 2024 22:25:28 GMT).

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.

@jongund rather than add colortheme and displayoption to cspell, I think we should change the syntax to use either color-theme and display-option or colorTheme and displayOption. I am pretty sure either would get past cspell. What is your preference?

Also, a navigation region is getting wrapped around the skipto button. We don't want that. We need an option to suppress. Or, better yet, the default should be to not add it. Skipto is the first thing on the page. The nav region is not useful in practice, and it adds extra verbosity that is distracting when tabbing.

@jongund
Copy link
Contributor Author

jongund commented Sep 21, 2023

The keywords are case insenstative, so the following updated configuration should fix the cspell issue and the navigation landmark problem is fixed by adding the containerElement option.

  <script data-skipto="colorTheme:aria; displayOption:popup; containerElement:div" src="../../shared/js/skipto.js"></script>

@mcking65
Copy link
Contributor

Thanks @jongund. That solved the problems.

This PR now provides the updated skipto with appropriate behaviors on all pages except for three:

  1. APG Home
  2. Patterns | APG | WAI | W3C
  3. Practices | APG | WAI | W3C

Since #2702, is not yet done, we will need to address the update of skipto for these three pages in wai-aria-practices via w3c/wai-aria-practices#259. We need to determine how that will be resolved before we merge this.

@a11ydoer, I'm assigning you as a reviewer to look at the visual presentation and mouse behavior of this version to make sure there are no regressions or unexpected changes.

@mcking65 mcking65 requested a review from a11ydoer September 21, 2023 17:38
@mcking65 mcking65 changed the title updated skipto.js to current distribution Infrastructure: update skipto.js to version 5.2.1 Sep 21, 2023
Copy link
Contributor

@a11ydoer a11ydoer left a comment

Choose a reason for hiding this comment

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

Navigation with keyboard only or mouse works.

@howard-e
Copy link
Contributor

howard-e commented Feb 6, 2024

@mcking65 @a11ydoer @jongund this PR has been updated with the suggested change to patterns.html and practices.html to include the data-skipto attributes in the skipto script import for those pages.

The preview link has also been rebuilt.

@howard-e howard-e requested review from mcking65 and a11ydoer February 6, 2024 22:30
@mcking65 mcking65 merged commit 5446c67 into main Feb 13, 2024
14 checks passed
@mcking65 mcking65 deleted the update/skipto branch February 13, 2024 06:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants