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

Creating Skip Navigation Links #29

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
Draft

Creating Skip Navigation Links #29

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
a991a7c
Creating Skip Navigation Links
arnvvd Oct 21, 2024
79beb10
Update skiplink selector
arnvvd Oct 21, 2024
6292998
Update skip link event listener
arnvvd Oct 21, 2024
3749d60
Update SkipLink.ts
arnvvd Oct 21, 2024
8958c57
Update SkipLink.ts
arnvvd Oct 21, 2024
51de6f0
Update src/components/SkipLink/SkipLink.ts
arnvvd Oct 21, 2024
34b6a1b
Apply skip links feedbacks
arnvvd Oct 21, 2024
dd60aa5
Update `SkipLink` default positioning to `absolute` instead of `fixed`
Jerek0 Oct 23, 2024
ec03f37
Merge branch 'main' into feature/skiplink
Jerek0 Oct 23, 2024
2e38251
Add swup-a11y-plugin & add #page-heading class to all `h1` occurrences
Jerek0 Nov 6, 2024
f60c33a
Merge branch 'feature/swup-a11y-plugin' into feature/skiplink
Jerek0 Nov 7, 2024
7954a15
[SkipLink] Use native anchor link instead of WebComponent + Add local…
Jerek0 Nov 7, 2024
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
23 changes: 23 additions & 0 deletions src/components/SkipLink/SkipLink.astro
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
---
import Button from '@components/Button/Button.astro';

interface Props {
label?: string;
class?: string;
target?: string;
}

const { label = 'Skip to main content', target, class: propsClass, ...rest } = Astro.props;

const classes = ['c-skip-link', propsClass];
---

<c-skip-link class:list={classes} target={target} {...rest}>
<Button>{label}</Button>
</c-skip-link>

<style is:global>
@import 'SkipLink.scss';
</style>

<script src="./SkipLink.ts"></script>
27 changes: 27 additions & 0 deletions src/components/SkipLink/SkipLink.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// ==========================================================================
// Components / Skip Link Button
// ==========================================================================

.c-skip-link {
position: fixed;
top: theme-spacing('fluid-sm');
left: 50%;
opacity: 0;
transform: translate3d(-50%, -100%, 0);
z-index: theme-z('skip');
white-space: nowrap;
pointer-events: none;
transition:
opacity theme-speed() theme-ease(),
transform theme-speed() theme-ease();

@media (prefers-reduced-motion) {
transition: none;
}

&:focus-within {
pointer-events: all;
opacity: 1;
transform: translate3d(-50%, 0, 0);
}
}
74 changes: 74 additions & 0 deletions src/components/SkipLink/SkipLink.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
export default class SkipLink extends HTMLElement {
private $button: ChildNode | null;

constructor() {
super();

// Binding
this.onClick = this.onClick.bind(this);
Copy link
Member

Choose a reason for hiding this comment

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

Rebinding is unnecessary if you declare your method as an arrow function:

- onClick(e: Event) {
+ onClick: (e: Event) => {

Copy link
Member

Choose a reason for hiding this comment

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

To my understanding that syntax is not supported inside of a class. Unless i'm missing something?

This would work if we moved the function inside the constructor but I'm not found of the idea

Copy link
Member

@mcaskill mcaskill Oct 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not fond of moving it to the constructor either.

It does work within classes as a class field. It links this to the instance. I've used it in personal projects and works fine as far as I can tell.

Copy link
Member

Choose a reason for hiding this comment

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

Oh so you meant

- onClick: (e: Event) => {
+ onClick = (e: Event) => {

Indeed that works, thanks for the tip!

Copy link
Member

Choose a reason for hiding this comment

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

Oh 🤦 Yes. = and not :.


// UI
this.$button = this.firstElementChild;
}

// =============================================================================
// Lifecycle
// =============================================================================
connectedCallback() {
this.bindEvents();
}

disconnectedCallback() {
this.unbindEvents();
}

// =============================================================================
// Events
// =============================================================================
bindEvents() {
console.log(this.$button);

if (this.$button) {
this.$button.addEventListener('click', this.onClick);
}
}
unbindEvents() {
if (this.$button) {
this.$button.removeEventListener('click', this.onClick);
}
}

// =============================================================================
// Callbacks
// =============================================================================
onClick(e: Event) {
e.preventDefault();

const $mainContent = document.querySelector(
Copy link
Member

Choose a reason for hiding this comment

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

This variable should be renamed to $target to match corresponding custom target attribute.

this.parentElement?.getAttribute('target') ?? 'main[tabindex]'
Copy link
Member

Choose a reason for hiding this comment

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

I recommend renaming the target attribute to href to avoid repurposing the concept of the existing target attribute.

Also, the fallback should be main instead of main[tabindex] since eitherway, there should only be one <main>.

) as HTMLElement;

if ($mainContent) {
$mainContent.scrollIntoView({
behavior: 'smooth',
block: 'start'
});
$mainContent.focus();

if (document.activeElement != $mainContent) {
$mainContent.setAttribute('tabindex', '-1');
$mainContent.focus();

$mainContent.addEventListener(
'blur',
() => {
$mainContent.removeAttribute('tabindex');
},
{ once: true }
);
}
}
}
}

customElements.define('c-skip-link', SkipLink);
5 changes: 4 additions & 1 deletion src/layouts/Layout.astro
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import '@styles/main.scss';
import { SEO } from 'astro-seo';
import { defaultSeo } from '@data/seo';
import { getSeo } from '@scripts/utils/seo';
import SkipLink from '@components/SkipLink/SkipLink.astro';

interface Props {
title: string;
Expand Down Expand Up @@ -43,6 +44,8 @@ const FONTS: string[] = [
}
</head>
<body>
<SkipLink />

<div class="u-container">
<nav class="u-flex u-gap-gutter u-py-fluid-md">
<a href="/">Index</a>
Expand All @@ -51,7 +54,7 @@ const FONTS: string[] = [
</nav>
</div>

<main role="main" id="swup" class="transition-fade">
<main role="main" id="swup" class="transition-fade" tabindex="-1">
<div id="content">
<slot />
</div>
Expand Down
1 change: 1 addition & 0 deletions tailwind.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ export default {
// smooth: 'cubic-bezier(0.380, 0.005, 0.215, 1)',
},
zIndex: {
skip: '300',
modal: '200',
header: '100',
above: '1',
Expand Down