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.

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

Focus broken when navigating with hash #3105

Closed
AnishDe12020 opened this issue Dec 24, 2021 · 5 comments · Fixed by #3177
Closed

Focus broken when navigating with hash #3105

AnishDe12020 opened this issue Dec 24, 2021 · 5 comments · Fixed by #3177
Labels
bug Something isn't working p2-nice-to-have SvelteKit cannot be used by a small number of people, quality of life improvements, etc. router

Comments

@AnishDe12020
Copy link

Describe the bug

In an effort to make tab navigation better on my site, I was implementing a skip to content button which would skip the navbar and directly go to the content. I have implemented it as it is normally implemented but I am running into a small problem. Normally, focussing on the skip to content button and then pressing enter gets you to the content, and pressing the tab key after that makes you cycle through the content. In my implementation, I am brought to the skip to content button twice before I can cycle through the site's content.

Reproduction

Add a skip to content link to a page (basically a link that is the first thing to come into focus and shows up when tabbed on landing on the website and else remains hidden). Set the href to an id the link points to (usually something like main or content that is after that page's navbar/header). Navigate to the site, press tab, press enter, again press tab and the skip to content link will again show up.

Logs

None. Check additional information for screen recording, details on code, source code, and a hosted site where it can be reproduced.

System Info

System:
    OS: Linux 5.15 Pop!_OS 21.10
    CPU: (8) x64 Intel(R) Core(TM) i7-8565U CPU @ 1.80GHz
    Memory: 3.19 GB / 15.52 GB
    Container: Yes
    Shell: 5.8 - /usr/bin/zsh
  Binaries:
    Node: 16.9.1 - ~/.nvm/versions/node/v16.9.1/bin/node
    Yarn: 1.22.17 - /usr/bin/yarn
    npm: 7.21.1 - ~/.nvm/versions/node/v16.9.1/bin/npm
  Browsers:
    Brave Browser: nightly
    Chrome: 96.0.4664.110
    Firefox: 95.0.1
  npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.3 
    @sveltejs/kit: next => 1.0.0-next.201 
    svelte: ^3.44.0 => 3.44.2

Severity

serious, but I can work around it

Additional Information

Hosted Preview - https://portfolio-nlaow838l-anishde12020.vercel.app/
Source code (THIS SPECIFIC COMMIT) - AnishDe12020/portfolio@d2406ff

Details on the code I have added a button to the layout of my website, that is in `__layout.svelte`. Here is the code:
<script>
  import "../app.css";
  import WipBanner from "../components/wip-banner.svelte";
  import Header from "../components/header.svelte";
  import Footer from "../components/footer.svelte";
  import BackToTop from "../components/back-to-top.svelte";
  import Socials from "../components/socials.svelte";
  import data from "../data.json";
</script>

<div class="max-w-[1920px] mx-auto">
  <a href="#content" class="skip-content-link">Skip To Content</a> <!-- the skip to content button -->
  <BackToTop />
  <WipBanner />
  <Header />
  <slot />
  <Footer />
  <Socials
    socialsData={data.socials}
    classNames="fixed bottom-8 left-8 flex-col space-y-6 hidden md:flex"
    showText
  />
</div>

I have added some styles in app.css for the same:

.skip-content-link {
  position: absolute;
  transform: translateY(-100%);
  background: #1d3dae;
  padding: 1rem 0.5rem;
  border-radius: 0rem 0rem 1rem 0rem;
}

.skip-content-link:focus {
  transform: translateY(0%);
}

At last, I have given the content an id of content in index.svelte so that it can be scrolled down to whenever the skip to content button is engaged:

<main id="content">
  <Hero />
  <BlogPosts blogPosts={data.blogPosts} blogBaseUrl={data.blogBaseUrl} />
  <Projects projects={data.projects} />
  <Skills skillsData={data.skills} />
  <Contact emailAddress={data.emailAddress} />
</main>

Note: I was following this article on css-tricks: https://css-tricks.com/how-to-create-a-skip-to-content-link/

For more context, here is a screen recording -

2021-12-23.22-50-58.mp4

Also, I have tried this on another SvelteKit project and had faced the same issue.

Discussion we are coming from - #3098

Currently using a workaround as suggested in sveltejs/sites#245

@bluwy bluwy added bug Something isn't working p3-edge-case SvelteKit cannot be used in an uncommon way router labels Dec 25, 2021
@jasonlyu123
Copy link
Member

It seems to be because we preventeDefault even when it's only navigating by hash.

event.preventDefault();

This prevented browser's default sequential focus navigation starting point behaviour. Don't call preventDefault for same-page navigation could fix this specific case.

But it won't work if the target is on another page(otherpage#hi). It would also be a problem when the app doesn't have SSR and the page started with a hashed URL like /somepath#i-am-an-id.

Wondering if there's any prior art in other frameworks regarding this. Would like it to work like traditional page navigation. I don't know any dom API that could do this. And we can't just set focus on the target element. As the element might not be focusable and if we make it focusable it might show a focus ring. The only thing I could think of now is to just create a dummy element, insert before the target element, focus it and remove it.

@bluwy
Copy link
Member

bluwy commented Dec 31, 2021

Looks like we're already detecting "hash navigation with no url change" scenarios at:

const i1 = url_string.indexOf('#');
const i2 = location.href.indexOf('#');
const u1 = i1 >= 0 ? url_string.substring(0, i1) : url_string;
const u2 = i2 >= 0 ? location.href.substring(0, i2) : location.href;
history.pushState({}, '', url.href);
if (u1 === u2) {
window.dispatchEvent(new HashChangeEvent('hashchange'));
}

I don't remember why we decided to emit a HashChangeEvent, instead of just returning so it defaults to the browser behaviour. The history.pushState({}, '', url.href); looks extraneous too. @benmccann Is there a reason for the way it is now? Those changes are introduced at #2590.

@benmccann
Copy link
Member

#2588 was the motivation for adding HashChangeEvent. I guess you're suggesting we could instead do:

 if (u1 === u2) { 
 	return; 
 } 
 history.pushState({}, '', url.href); 

I don't think we really considered that. If that works better I think we could change it. Though according to @jasonlyu123's comment above #3105 (comment) I'm not sure that would fix all cases

@geoffrich
Copy link
Member

As a note, this affects all navigation by hash, not just skip-to-content links, which makes it much less of an edge case. For instance, if I'm navigating by keyboard around the SvelteKit docs and hit enter on a link to another section of the page (e.g. https://kit.svelte.dev/docs#configuration), the page scrolls to the relevant section, but my focus is reset to the top of the page. This requires me to tab forever to reach the section I was linked to, if I can even find it. This also means that if I'm navigating the docs with a screen reader, all the intra-page links are unusable, since they'll make me lose my spot and reset my position to the top of the page.

Essentially, this bug makes experiences like the SvelteKit docs (with lots of hash links inside the page) inaccessible to screen reader and keyboard-only users.

@benmccann benmccann added p2-nice-to-have SvelteKit cannot be used by a small number of people, quality of life improvements, etc. and removed p3-edge-case SvelteKit cannot be used in an uncommon way labels Dec 31, 2021
@benmccann benmccann changed the title Skip To Content button doesn't work properly Focus broken when navigating with hash Dec 31, 2021
@bluwy
Copy link
Member

bluwy commented Jan 1, 2022

Though according to @jasonlyu123's comment above #3105 (comment) I'm not sure that would fix all cases

But it won't work if the target is on another page(otherpage#hi. It would also be a problem when the app doesn't have SSR and the page started with a hashed URL like /somepath#i-am-an-id.

If we're navigating to another page with a hash, or a page without SSR, I think we had already covered that together with the scrolling logic when we navigate to the new page. I think the only case where this wouldn't work is if we intentionally disable the router, which would be expected behaviour. Might need to double check this though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p2-nice-to-have SvelteKit cannot be used by a small number of people, quality of life improvements, etc. router
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants