init: add skip-to-main-content shortcut #7390

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 <[email protected]>

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.

apps/site/components/withNavBar.tsx

Signed-off-by: Yaten Dhingra <[email protected]>

apps/site/components/withNavBar.tsx

Signed-off-by: Yaten Dhingra <[email protected]>

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

apps/site/components/withNavBar.tsx


  const toggleCurrentTheme = () =>
    setTheme(resolvedTheme === 'dark' ? 'light' : 'dark');

  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.

apps/site/components/withNavBar.tsx

Signed-off-by: Yaten Dhingra <[email protected]>

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 <[email protected]>

yaten2302 · 2025-01-16T17:46:03Z

Hi, I've pushed a commit, and I think that this addresses all the changes mentioned and required! Could anyone please have a look at it and LMK if any change has to be made in this?

yaten2302 · 2025-01-19T14:32:48Z

Hey, can anyone please have a look and LMK if any more changes are required?

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.

apps/site/components/withNavBar.tsx

Signed-off-by: Yaten Dhingra <[email protected]>

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

apps/site/components/withLayout.tsx

@@ -19,6 +21,7 @@ const layouts = {
  '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?

apps/site/components/withNavBar.tsx

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

apps/site/types/layouts.ts

@@ -6,4 +6,5 @@ export type Layouts =
  | 'blog-category'
  | 'blog-post'
  | 'download'
-  | 'article';
+  | 'article'
+  | 'skipToContent';


Similarly, IMO there should be no skip to content layout

Signed-off-by: Yaten Dhingra <[email protected]>

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.

init: add skip-to-main-content shortcut

bfd4bbd

Signed-off-by: Yaten Dhingra <[email protected]>

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

Open

vercel bot deployed to Preview January 7, 2025 06:58 View deployment

AugustinMauroy requested changes Jan 7, 2025

View reviewed changes

apps/site/components/withNavBar.tsx Show resolved Hide resolved

apps/site/components/withNavBar.tsx Outdated Show resolved Hide resolved

apps/site/components/withNavBar.tsx Outdated Show resolved Hide resolved

fix imports in withNavBar.tsx

85e3e14

Signed-off-by: Yaten Dhingra <[email protected]>

vercel bot deployed to Preview January 7, 2025 08:50 View deployment

added classNames lib

af3799a

Signed-off-by: Yaten Dhingra <[email protected]>

vercel bot deployed to Preview January 7, 2025 09:01 View deployment

aduh95 requested changes Jan 7, 2025

View reviewed changes

apps/site/components/withNavBar.tsx Outdated Show resolved Hide resolved

canerakdas reviewed Jan 7, 2025

View reviewed changes

apps/site/components/withNavBar.tsx Outdated Show resolved Hide resolved

made skipToContent button CSS-only

0c9c1b5

Signed-off-by: Yaten Dhingra <[email protected]>

vercel bot deployed 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

vercel bot had a problem deploying to Preview January 10, 2025 14:14 Failure

changed classNameOnTabPress to skipToContent

0b24e44

Signed-off-by: Yaten Dhingra <[email protected]>

vercel bot deployed to Preview January 10, 2025 14:21 View deployment

Merge branch 'nodejs:main' into main

c05f0b9

vercel bot deployed to Preview January 11, 2025 17:42 View deployment

Merge branch 'nodejs:main' into main

b5f2322

vercel bot deployed to Preview January 15, 2025 18:13 View deployment

added skipToContent in withLayout.tsx

536bfd7

Signed-off-by: Yaten Dhingra <[email protected]>

vercel bot deployed to Preview January 15, 2025 18:18 View deployment

Merge branch 'nodejs:main' into main

61d34c8

vercel bot deployed to Preview January 16, 2025 17:34 View deployment

added appropriate class to <a/> tag

db8543f

Signed-off-by: Yaten Dhingra <[email protected]>

vercel bot deployed to Preview January 16, 2025 17:45 View deployment

AugustinMauroy reviewed Jan 19, 2025

View reviewed changes

apps/site/components/withNavBar.tsx Outdated Show resolved Hide resolved

updated skitToContent classname

2a465e3

Signed-off-by: Yaten Dhingra <[email protected]>

vercel bot deployed 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

Open

5 tasks

minor: remove custom className skipToContent

2233341

Signed-off-by: Yaten Dhingra <[email protected]>

vercel bot deployed to Preview January 24, 2025 15:12 View deployment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

init: add skip-to-main-content shortcut #7390

init: add skip-to-main-content shortcut #7390

yaten2302 commented Jan 7, 2025

vercel bot commented Jan 7, 2025 •

edited

Loading

yaten2302 commented Jan 7, 2025 •

edited

Loading

AugustinMauroy commented Jan 7, 2025

araujogui Jan 8, 2025

yaten2302 Jan 9, 2025

araujogui Jan 13, 2025

yaten2302 Jan 15, 2025

araujogui Jan 16, 2025

yaten2302 Jan 17, 2025

araujogui Jan 20, 2025

yaten2302 commented Jan 10, 2025

yaten2302 commented Jan 16, 2025

yaten2302 commented Jan 19, 2025

AugustinMauroy commented Jan 19, 2025 •

edited

Loading

aduh95 commented Jan 22, 2025

yaten2302 commented Jan 23, 2025

aduh95 commented Jan 23, 2025

yaten2302 commented Jan 23, 2025

canerakdas Jan 23, 2025

yaten2302 Jan 24, 2025

canerakdas Jan 23, 2025

aduh95 commented Jan 24, 2025

yaten2302 commented Jan 24, 2025

aduh95 commented Jan 24, 2025

yaten2302 commented Jan 25, 2025

yaten2302 commented Jan 25, 2025

init: add skip-to-main-content shortcut #7390

Are you sure you want to change the base?

init: add skip-to-main-content shortcut #7390

Conversation

yaten2302 commented Jan 7, 2025

Description

Related Issues

Check List

vercel bot commented Jan 7, 2025 • edited Loading

yaten2302 commented Jan 7, 2025 • edited Loading

AugustinMauroy commented Jan 7, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yaten2302 commented Jan 10, 2025

yaten2302 commented Jan 16, 2025

yaten2302 commented Jan 19, 2025

AugustinMauroy commented Jan 19, 2025 • edited Loading

aduh95 commented Jan 22, 2025

yaten2302 commented Jan 23, 2025

aduh95 commented Jan 23, 2025

yaten2302 commented Jan 23, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aduh95 commented Jan 24, 2025

yaten2302 commented Jan 24, 2025

aduh95 commented Jan 24, 2025

yaten2302 commented Jan 25, 2025

yaten2302 commented Jan 25, 2025

vercel bot commented Jan 7, 2025 •

edited

Loading

yaten2302 commented Jan 7, 2025 •

edited

Loading

AugustinMauroy commented Jan 19, 2025 •

edited

Loading