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 7 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} {...rest} data-no-swup>
<Button href="#main">{label}</Button>
</c-skip-link>
Copy link
Member

Choose a reason for hiding this comment

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

A Web site can have more than one skip link that target different locations in a document. We could reasonably assume that the first, and if only, skip link is one that targets a #main element.

Suggested change
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} {...rest} data-no-swup>
<Button href="#main">{label}</Button>
</c-skip-link>
interface Props {
label?: string;
class?: string;
href?: string;
}
const { label = 'Skip to main content', href = '#main', class: propsClass, ...rest } = Astro.props;
const classes = ['c-skip-link', propsClass];
---
<c-skip-link class:list={classes} {...rest} data-no-swup>
<Button href={href}>{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);
}
}
71 changes: 71 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,71 @@
export default class SkipLink extends HTMLElement {
private $link: HTMLLinkElement | 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.$link = this.firstElementChild as HTMLLinkElement;
Copy link
Member

Choose a reason for hiding this comment

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

I don't know how Astro handles custom elements but with native custom elements, you can't expect an element's children to be available during the constructor (unless maybe everything is executed after DOMContentLoaded).

Copy link
Member

Choose a reason for hiding this comment

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

Web components defined inside of an Astro component are loaded & executed after the main app.ts file, which I'm almost certain is executed after DOMContentLoaded as you suspected, so it shouldn't break

That being said, we could indeed move that assignation to the connectedCallback function instead just to be safe.

}

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

disconnectedCallback() {
this.unbindEvents();
}

// =============================================================================
// Events
// =============================================================================
bindEvents() {
if (this.$link) {
this.$link.addEventListener('click', this.onClick);
}
}
unbindEvents() {
if (this.$link) {
this.$link.removeEventListener('click', this.onClick);
}
}

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

const $target = document.querySelector(
this.$link?.getAttribute('href') ?? 'main'
) as HTMLElement;
Copy link
Member

Choose a reason for hiding this comment

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

For consistency with the default value of href in SkipLink.astro, this should also target #main. Using a comma to define multiple selectors allows the skip link to find the first available match.

Suggested change
const $target = document.querySelector(
this.$link?.getAttribute('href') ?? 'main'
) as HTMLElement;
const $target = document.querySelector(
this.$link?.getAttribute('href') ?? '#main,main'
) as HTMLElement;


if (!$target) {
return;
}

$target.scrollIntoView({
behavior: 'smooth',
block: 'start'
});

$target.focus();

if (document.activeElement === $target) {
return;
}

$target.setAttribute('tabindex', '-1');
$target.focus();

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

customElements.define('c-skip-link', SkipLink);
7 changes: 5 additions & 2 deletions 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,8 +54,8 @@ const FONTS: string[] = [
</nav>
</div>

<main role="main" id="swup" class="transition-fade">
<div id="content">
<main id="swup" class="transition-fade">
<div id="main">
<slot />
</div>
</main>
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