init: add skip-to-main-content shortcut by yaten2302 · Pull Request #7390 · nodejs/nodejs.org

yaten2302 · 2025-01-07T06:57:02Z

Description

This PR adds a skip to main content button (initially hidden), when the user presses Tab(for windows), the button is visible and the user can then move directly to the main content of the page.

Related Issues

Fixes#7220

Check List

I have read the Contributing Guidelines and made commit messages that follow the guideline.
I have run npm run format to ensure the code follows the style guide.
I have run npm run test to check if all tests are passing.
I have run npx turbo build to check if the website builds without errors.
I've covered new added functionality with unit tests if necessary.

Signed-off-by: Yaten Dhingra <yaten598@gmail.com>

vercel · 2025-01-07T06:57:07Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
nodejs-org	✅ Ready (Inspect)	Visit Preview	Jan 24, 2025 3:12pm

yaten2302 · 2025-01-07T07:38:23Z

I think currently on pressing Tab, the button is shown, but it's not getting pressed if the user presses Enter, I forgot take into account the Enter button, I'll push a commit for this too 👍

Edit: Sorry, will not ping 👍

AugustinMauroy · 2025-01-07T08:33:13Z

WOW stop pinging everyone ! we (website team) receive notification when there are changes on repo.

Signed-off-by: Yaten Dhingra <yaten598@gmail.com>

araujogui · 2025-01-08T15:09:40Z


 return (
 <div>
+ <a className={classNameOnTabPress} href="#main">


Anchor href is not working, main element is missing the id attribute.

This href won't work as of now, because I just set it randomly to #main, I wanted to clarify that to what section the user must be directed to on clicking this button?

I believe It should redirect to main indeed

For this, is there a main class present in the pages? Ig not?
There is this <main/> tag present in the about page, just for example.

Sorry, I don't understand what you saying

I was trying to say that on any page, since we're redirecting to #main, there is no such class as #main on any page, so where should the user be redirected?

User should be "redirected" to main, layouts lives on apps/site/layouts.

Signed-off-by: Yaten Dhingra <yaten598@gmail.com>

yaten2302 · 2025-01-10T14:24:38Z

I've pushed a minor change(typo). Just wanted to ask, that could anyone please guide regarding these points, I was having a few doubts:

Signed-off-by: Yaten Dhingra <yaten598@gmail.com>

AugustinMauroy · 2025-01-19T14:54:40Z

I don't really know what to do with this button, but at the moment it degrades the UI.

Signed-off-by: Yaten Dhingra <yaten598@gmail.com>

aduh95 · 2025-01-22T18:16:06Z

I've tried to pull that PR on top of main and run npm run dev but couldn't get the button to show up, am I missing something?

yaten2302 · 2025-01-23T05:44:48Z

2025-01-23.11-09-38.mp4

I've shared a screen recording for the same, is this behavior not being shown on your side? I just checked that in the deployment also it's showing this.

aduh95 · 2025-01-23T10:19:07Z

No even running from the DevTools Array.from(document.all).find(a => a.textContent.trim() === 'Skip to content') returns undefined 🤔

My steps are:

git fetch https://github.com/nodejs/nodejs.org.git HEAD
git reset FETCH_HEAD --hard
curl -L https://github.com/nodejs/nodejs.org/pull/7390.patch | git am
npm ci
npm run dev

yaten2302 · 2025-01-23T11:42:16Z

I used deployment dev tools:

canerakdas · 2025-01-23T16:51:49Z

 'blog-category': BlogLayout,
 download: DownloadLayout,
 article: ArticlePageLayout,
+ skipToContent: WithNavBar,


I don't think skipToContent is a page layout, IMO it is a feature in layouts

Someone mentioned about this in previous suggestions, that it needs to be added in withLayout.tsx, should I remove this?

I think you are talking about this comment; #7390 (comment)
@araujogui As far as I understand, the suggestion is to move the skip to content button here, not add it as a layout right? 🤔

Yes, saying about this 👍

I think you are talking about this comment; #7390 (comment)
@araujogui As far as I understand, the suggestion is to move the skip to content button here, not add it as a layout right? 🤔
Yep, that's what I meant

canerakdas · 2025-01-23T17:20:47Z

 | 'download'
- | 'article';
+ | 'article'
+ | 'skipToContent';


Similarly, IMO there should be no skip to content layout

Signed-off-by: Yaten Dhingra <yaten598@gmail.com>

aduh95 · 2025-01-24T17:45:48Z

It looks like we don't have any element with a main id, so the button is moot.

I've also tried to use the website using VoiceOver, it seems completely impossible to access the menu from there, which is pretty bad, but I guess I'll open a separate issue for that.

yaten2302 · 2025-01-24T18:04:50Z

Yes, I think, the desired behavior is such that on clicking skip to content button, the user should be redirected to the section with id - #main, right?
And I think it's not present as of now, so once all the ID's are added, then it would be helpful, am I right?

aduh95 · 2025-01-24T21:17:36Z

As you can read in https://www.w3.org/TR/WCAG20-TECHS/G1.html#G1-tests, in order to fix the linked issue the following checks need to pass:

[…]
4. Check that activating the link moves the focus to the main content.
5. Check that after activating the link, the keyboard focus has moved to the main content.

Currently those check fail; adding the ids might fix that yes

yaten2302 · 2025-01-25T05:43:52Z

May I create an issue for the same?
Something like - Add #main id to navigate to main section of pages?

yaten2302 · 2025-01-25T05:45:20Z

By the time this issue gets fixed, I would like to work upon some other issues too! And this PR can be kept on hold for a while till the time the main id is added in all pages.

ovflowd

I'd ask to first apply the suggestions made from other users. I appreciate that you're iterating over our feedback and appreciate your time here.

Having that said, I don't think this can land as it is right now. The button is simply hiding other content and overlapping with it in a non-productive manner.

I'd recommend using the small variant of our Button, and making it floating on the bottom right (position: fixed) or something of sort.

That said, I don't really think our pages need a "skip to content" Button -- Our pages are already designed in a way that the content is always already present even when scroll is zeroed. It is a very responsive design and that does not require a button to skip to content, since again, the content is always visible.

I'm -1 on this change.

aduh95 · 2025-01-26T17:15:07Z

For example the download page could definitely benefit form it, it takes a long while to get to the download button using VoiceOver – but ~~that could be solved by adding a low tabindex attribute to the download button(s)~~ EDIT: I was wrong, messing with tabindex is listed as an anti-pattern

ovflowd · 2025-01-26T17:18:51Z

For example the download page could definitely benefit form it, it takes a long while to get to the download button using VoiceOver – but that could be solved by adding a low tabindex attribute to the download button(s).

Which download page? The download page renders all in one screen without even needing to scroll

ovflowd · 2025-01-26T17:21:40Z

Also the whole downloads page is the main content; A skip to main content there just does not make any sense here.

aduh95 · 2025-01-26T17:25:24Z

For example the download page could definitely benefit form it, it takes a long while to get to the download button using VoiceOver – but that could be solved by adding a low tabindex attribute to the download button(s).
Which download page? The download page renders all in one screen without even needing to scroll

VoiceOver is a screen reader built into Apple OSes (macOS, iOS, tvOS, watchOS, iPadOS) designed to increase accessibility for blind and low-vision users, as well as for users with dyslexia. The fact that all the content is visible without scrolling is irrelevant for folks who can't see

ovflowd · 2025-01-26T21:24:49Z

For example the download page could definitely benefit form it, it takes a long while to get to the download button using VoiceOver – but that could be solved by adding a low tabindex attribute to the download button(s).
Which download page? The download page renders all in one screen without even needing to scroll
VoiceOver is a screen reader built into Apple OSes (macOS, iOS, tvOS, watchOS, iPadOS) designed to increase accessibility for blind and low-vision users, as well as for users with dyslexia. The fact that all the content is visible without scrolling is irrelevant for folks who can't see

Are we talking about the same thing? I'm talking about a skip to content button. That doesn't make sense to be rendered nor added. We can detect by CSS queries if the user is using a VoiceOver tool, in those situations if you believe any extra accessibility accessory needs to be added here, I'd like to follow standards proposed by a11y.

aduh95 · 2025-01-26T21:36:48Z

As you can read in #7220 description (or directly in https://www.w3.org/TR/WCAG20-TECHS/G1.html#G1-tests), the "Skip to content" button is an a11y best practice. Anyway, your point of "we may actually not need it" is not invalid, I'm not disputing it – though I'd encourage you to try browsing the Download page with a screen reader, getting to the important links takes a frustrating long time.

ovflowd · 2025-01-26T22:06:07Z

As you can read in #7220 description (or directly in w3.org/TR/WCAG20-TECHS/G1.html#G1-tests), the "Skip to content" button is an a11y best practice. Anyway, your point of "we may actually not need it" is not invalid, I'm not disputing it – though I'd encourage you to try browsing the Download page with a screen reader, getting to the important links takes a frustrating long time.

Noted. I believe this is something tricky. I would consider closing this PR and open an discussion first so we can properly discuss about how to approach this. Maybe for key/essential pages we can have invisible elements that are rendered for screen readers, that's also a common practice.

yaten2302 · 2025-01-27T06:23:52Z

Should I close this PR for now? Maybe after some discussion, a new PR can be created?

ovflowd · 2025-01-28T20:08:38Z

Should I close this PR for now? Maybe after some discussion, a new PR can be created?

I think that would be the best move for the time being. I appreciate your time here 🙏

aduh95 · 2025-01-28T21:55:39Z

Another move would be to change this PR to only add the button to the Learn pages, which would definitely benefit from such button – but it needs to actually work, we can't add a button that does nothing.

yaten2302 · 2025-01-30T05:58:54Z

Thanks ovflowd 👍
Will work on some other issues for the time being 👍

init: add skip-to-main-content shortcut
bfd4bbd
Signed-off-by: Yaten Dhingra <yaten598@gmail.com>

yaten2302 requested a review from a team as a code owner January 7, 2025 06:57

yaten2302 mentioned this pull request Jan 7, 2025
Adding a "Skip to Content" Link #7220
Closed

vercelBotdeployed to Preview January 7, 2025 06:58 View deployment

AugustinMauroy requested changes Jan 7, 2025
View reviewed changes

Comment threadapps/site/components/withNavBar.tsx
Comment threadapps/site/components/withNavBar.tsx Outdated
Comment threadapps/site/components/withNavBar.tsx Outdated

fix imports in withNavBar.tsx
85e3e14
Signed-off-by: Yaten Dhingra <yaten598@gmail.com>

vercelBotdeployed to Preview January 7, 2025 08:50 View deployment

added classNames lib
af3799a
Signed-off-by: Yaten Dhingra <yaten598@gmail.com>

vercelBotdeployed to Preview January 7, 2025 09:01 View deployment

aduh95 requested changes Jan 7, 2025
View reviewed changes

Comment threadapps/site/components/withNavBar.tsx Outdated

canerakdas reviewed Jan 7, 2025
View reviewed changes

Comment threadapps/site/components/withNavBar.tsx Outdated

made skipToContent button CSS-only
0c9c1b5
Signed-off-by: Yaten Dhingra <yaten598@gmail.com>

vercelBotdeployed to Preview January 7, 2025 17:28 View deployment

araujogui requested changes Jan 8, 2025
View reviewed changes

Merge branch 'nodejs:main' into main
9f14df0

vercelBothad a problem deploying to Preview January 10, 2025 14:14 Failure

changed classNameOnTabPress to skipToContent
0b24e44
Signed-off-by: Yaten Dhingra <yaten598@gmail.com>

vercelBotdeployed to Preview January 10, 2025 14:21 View deployment

Merge branch 'nodejs:main' into main
c05f0b9

vercelBotdeployed to Preview January 11, 2025 17:42 View deployment

Merge branch 'nodejs:main' into main
b5f2322

vercelBotdeployed to Preview January 15, 2025 18:13 View deployment

added skipToContent in withLayout.tsx
536bfd7
Signed-off-by: Yaten Dhingra <yaten598@gmail.com>

vercelBotdeployed to Preview January 15, 2025 18:18 View deployment

Merge branch 'nodejs:main' into main
61d34c8

vercelBotdeployed to Preview January 16, 2025 17:34 View deployment

AugustinMauroy reviewed Jan 19, 2025
View reviewed changes

Comment threadapps/site/components/withNavBar.tsx Outdated

updated skitToContent classname
2a465e3
Signed-off-by: Yaten Dhingra <yaten598@gmail.com>

vercelBotdeployed to Preview January 22, 2025 17:53 View deployment

canerakdas reviewed Jan 23, 2025
View reviewed changes

canerakdas mentioned this pull request Jan 24, 2025
fix: Button accessibility #7420
Merged
5 tasks

minor: remove custom className skipToContent
2233341
Signed-off-by: Yaten Dhingra <yaten598@gmail.com>

vercelBotdeployed to Preview January 24, 2025 15:12 View deployment

ovflowd requested changes Jan 26, 2025
View reviewed changes

yaten2302 closed this Jan 30, 2025

Uh oh!

Conversation

yaten2302 commented Jan 7, 2025

Description

Related Issues

Check List

Uh oh!

vercelBot commented Jan 7, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yaten2302 commented Jan 7, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AugustinMauroy commented Jan 7, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yaten2302 commented Jan 10, 2025

Uh oh!

AugustinMauroy commented Jan 19, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

aduh95 commented Jan 22, 2025

Uh oh!

yaten2302 commented Jan 23, 2025

Uh oh!

aduh95 commented Jan 23, 2025

Uh oh!

yaten2302 commented Jan 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aduh95 commented Jan 24, 2025

Uh oh!

yaten2302 commented Jan 24, 2025

Uh oh!

aduh95 commented Jan 24, 2025

Uh oh!

yaten2302 commented Jan 25, 2025

Uh oh!

yaten2302 commented Jan 25, 2025

Uh oh!

ovflowd left a comment

Choose a reason for hiding this comment

Uh oh!

aduh95 commented Jan 26, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ovflowd commented Jan 26, 2025

vercelBot commented Jan 7, 2025•
edited
Loading

yaten2302 commented Jan 7, 2025•
edited
Loading

AugustinMauroy commented Jan 19, 2025•
edited
Loading

aduh95 commented Jan 26, 2025•
edited
Loading