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.

Sign up for GitHub

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

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
+19 −3
Open

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

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
bfd4bbd
init: add skip-to-main-content shortcut
yaten2302 Jan 7, 2025
85e3e14
fix imports in withNavBar.tsx
yaten2302 Jan 7, 2025
af3799a
added classNames lib
yaten2302 Jan 7, 2025
0c9c1b5
made skipToContent button CSS-only
yaten2302 Jan 7, 2025
9f14df0
Merge branch 'nodejs:main' into main
yaten2302 Jan 10, 2025
0b24e44
changed classNameOnTabPress to skipToContent
yaten2302 Jan 10, 2025
c05f0b9
Merge branch 'nodejs:main' into main
yaten2302 Jan 11, 2025
b5f2322
Merge branch 'nodejs:main' into main
yaten2302 Jan 15, 2025
536bfd7
added skipToContent in withLayout.tsx
yaten2302 Jan 15, 2025
61d34c8
Merge branch 'nodejs:main' into main
yaten2302 Jan 16, 2025
db8543f
added appropriate class to <a/> tag
yaten2302 Jan 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion apps/site/components/withNavBar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use client';

import { useLocale } from 'next-intl';
import classNames from 'classnames';
import { useLocale, useTranslations } from 'next-intl';
import { useTheme } from 'next-themes';
import type { FC } from 'react';
yaten2302 marked this conversation as resolved.
Show resolved Hide resolved

Expand All @@ -16,13 +17,23 @@ const WithNavBar: FC = () => {
const { replace } = useRouter();
const pathname = usePathname();

const classNameOnTabPress = classNames(
Copy link
Member

Choose a reason for hiding this comment

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

I believe that creating a SkipToContentButton component would make this cleaner.

Copy link
Member

Choose a reason for hiding this comment

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

And I feel that WithLayout is a better place to put this component.

Copy link
Author

Choose a reason for hiding this comment

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

Should I change classNameOnTabPress to SkipToContentButton?

Copy link
Author

Choose a reason for hiding this comment

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

And I feel that WithLayout is a better place to put this component.

All these changes have to be shifted to the withLayout, am I right?

Copy link
Member

Choose a reason for hiding this comment

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

Should I change classNameOnTabPress to SkipToContentButton?

Yes

And I feel that WithLayout is a better place to put this component.

All these changes have to be shifted to the withLayout, am I right?

Yep

Copy link
Author

Choose a reason for hiding this comment

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

If I've understood this correct, then should I add a new skipToContent in here?

Copy link
Author

Choose a reason for hiding this comment

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

Hey, apologies for delay!
I've pushed a commit to address this, could you please have a look at it?
Thanks

'bg-[#000] text-center font-semibold inline-flex items-center justify-center gap-2 py-2.5',
'absolute left-0 top-0 m-3 -translate-y-16 bg-blue-500 p-3 text-white transition-transform focus:translate-y-0 focus:outline-none'
);

const locale = useLocale();
const t = useTranslations();

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

return (
<div>
<a className={classNameOnTabPress} href="#main">
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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?

Copy link
Member

Choose a reason for hiding this comment

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

I believe It should redirect to main indeed

Copy link
Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I don't understand what you saying

Copy link
Author

Choose a reason for hiding this comment

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

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?

{t('components.containers.navBar.links.skipToContent')}
</a>

<WithBanner section="index" />

<NavBar
Expand Down
3 changes: 2 additions & 1 deletion packages/i18n/locales/en.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
"security": "Security",
"certification": "Certification",
"blog": "Blog",
"contribute": "Contribute"
"contribute": "Contribute",
"skipToContent": "Skip to content"
}
}
},
Expand Down
Loading