Skip to content

fix: resolve the failure of skip-to-content button - #665

Merged
satnaing merged 3 commits into
satnaing:mainfrom
hazuki-keatsu:fix/skip-to-content
Aug 5, 2026
Merged

fix: resolve the failure of skip-to-content button#665
satnaing merged 3 commits into
satnaing:mainfrom
hazuki-keatsu:fix/skip-to-content

Conversation

@hazuki-keatsu

Copy link
Copy Markdown
Contributor

Description

The existing skip-to-content button only used an anchor link to scroll to #main-content, but did not move keyboard/screen-reader focus to the main content. This meant that after activation, the focus remained on the skip link itself, failing to actually bypass the header navigation for assistive technology users.

This PR updates Header.astro to properly shift focus by adding a click handler for existing skip-to-content button.

Types of changes

  • Bug Fix (non-breaking change which fixes an issue)
  • New Feature (non-breaking change which adds functionality)
  • Documentation Update (if none of the other choices apply)
  • Others (any other types not listed above)

Checklist

  • I have read the Contributing Guide
  • I have added the necessary documentation (N/A)
  • Breaking Change (fix or feature that would cause existing functionality to not work as expected)

@satnaing satnaing left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for tracking this down. Nice catch.
I just left a couple of minor suggestions.

Comment thread src/components/Header.astro
Comment thread src/components/Header.astro Outdated

@satnaing satnaing left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Can you drop all mb-0.5 in this PR please?
I don't see any problem without having a bottom margin in these layout.

Also, can you add an outline styling to #main-content so that the outline style stays consistent across the site.

Image

You can just add #main-content in global.css just like this

@layer base {
  /* other styles */
  a,
  button,
  #main-content {
    @apply outline-accent outline-offset-1 focus-visible:no-underline focus-visible:outline-2 focus-visible:outline-dashed;
  }
  /* other styles */
}

@hazuki-keatsu

Copy link
Copy Markdown
Contributor Author

The Tags page may be a special example without any problem. But in the index page, the outline of main will be covered by the header and the footer: (The first image is from Firefox, and the second one is from Edge)

  • Tags:
    Firefox Tags

    Edge Tags
  • Index
    Firefox Main

    Edge Main

If you'd rather I not use mb-0.5 everywhere, I'll try your suggestion above.

- Guard skip link against modified clicks so new-tab opens still work
- Strip tabindex from <main> on blur
- Modify the scroll behavior from instant to smooth
- Add some margin to make the focus outline won't be covered.
- Beauty the outline style.
@hazuki-keatsu

Copy link
Copy Markdown
Contributor Author

Without additional margin:

Without my-1

With additional margin:

With my-1

@satnaing

satnaing commented Aug 4, 2026

Copy link
Copy Markdown
Owner

If we're sure that only one page is affected, in this case index src/pages/index.astro, it'd be better to add a vertical margin on that page alone.

Screenshot 2026-08-04 at 5 54 52 PM

Comment thread src/styles/global.css Outdated
@hazuki-keatsu

Copy link
Copy Markdown
Contributor Author

The only affected pages are src/pages/index.astro and src/pages/404.astro, as they do not use the Breadcrumb and BackButton components. These components add extra bottom margins (mb-1 and mb-2, respectively) which affect the outline spacing of the main element.

So, what's your preference? Would you rather add margins to each affected page individually, instead of applying a single my-1 class to all main-content elements?

@satnaing

satnaing commented Aug 5, 2026

Copy link
Copy Markdown
Owner

We'll just add extra margins (mb-0.5 if it's ok) in the affected pages.
Then we can update global.css according to my above suggested changes.

@hazuki-keatsu

Copy link
Copy Markdown
Contributor Author

OK. I added margins for the affected pages. I used my-1 not my-0.5, as I found the outline will be covered by Header and Footer partially. So I thought my-1 will be the right choice.

@satnaing

satnaing commented Aug 5, 2026

Copy link
Copy Markdown
Owner

Looks good.
But, I don't see any updates on global.css yet.

- Add extra margins for the pages without `Breadcrumb` and `BackButton`, instead of adding margin for all `main-content`s, making the outline of `main` not covered by `Header` and `Footer`.
@hazuki-keatsu

Copy link
Copy Markdown
Contributor Author

I'm very sorry, that was my fault. I forgot to make the changes to global.css. It should be fine now. 😸

@satnaing satnaing left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

LGTM, thanks.

@satnaing
satnaing merged commit 35cfa7f into satnaing:main Aug 5, 2026
1 check passed
charles-hood pushed a commit to charles-hood/cdh-astro-blog-v6 that referenced this pull request Aug 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants