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

Restore focus to persisted element after replacing content #901

Closed

Conversation

ovenum
Copy link

@ovenum ovenum commented Jun 6, 2024

This PR restores focus to a DOM Element persisted with data-swup-persist if it was the focused element before content replacing took place.

This is useful for input elements that trigger a search and should remain focused when the result is added to the DOM.

To further illustrate what this PR tries to solve please see the example on replit here

Checks

  • The PR is submitted to the master branch
  • The code was linted before pushing (npm run lint)
  • All tests are passing (npm run test)
  • New or updated tests are included
  • The documentation was updated as required

Additional information

  • Tests are missing and would appreciate help in adding them if this PR should be merged.
  • Documentation for data-swup-persist should reflect that focus is persisted.

@daun
Copy link
Member

daun commented Jun 6, 2024

Hi @ovenum 👋 Thanks for contributing! I agree about things working as you're suggesting for the input use case. I'd like to ponder this some more with other scenarios in mind, e.g. a persisted link element with a shopping cart counter which navigates to the shopping cart page. My feeling is that I'd like the focus here to be reset to the body as on normal visits. Maybe we can limit this to input or form elements in general. Or would this get too complex?

@daun
Copy link
Member

daun commented Jun 14, 2024

@hirasso What's your opinion on this? With just the core in mind, this sounds like a good idea, no? Where it confuses me is the a11y stuff — we'd 1) lose focus 2) refocus the input 3) reset focus to the body. Might be confusing, but it also might just be debounced and totally fine. I'd suggest limiting this to inputs to keep this specific.

@ovenum
Copy link
Author

ovenum commented Jun 14, 2024

There could be another issue with persisted elements having client state that is only tracked in JS and input elements inside these elements. For example a persisted section nav-bar with filter menus and a search input.

If focus is only re-applied to input elements, it could be enough to also search the child elements of the persisted element for input elements.

How would this work with WebComponents that extend on Input elements?

@hirasso
Copy link
Member

hirasso commented Jun 15, 2024

Thank you @ovenum for bringing this up!

I have recently been fighting with a very similar use case, as well.

I ended up not persisting my filter nav but rather managed as much state as possible on the server, and restored the focus using a custom hook and @swup/a11y-plugin (which provides custom handling of visit.a11y.focus):

swup.hooks.before("content:replace", (visit) => {
  const focusID = document.activeElement?.getAttribute("data-focus-id");
  if (focusID) {
    visit.a11y.focus = `[data-focus-id="${focusID}"]`;
  }
});

That way, I can control from my markup if the focus on an element should be restored when navigating:

<!-- restore focus to this element if it had focus before the visit -->
<input data-focus-id="unique-id" type="text"></input>

While thinking about focus, we have to keep in mind, that the expected behavior for screen reader users is that the focus is always set to the <body> for each new page visit. That's also what we did in version 5 of @swup/a11-plugin.

If we decide to proceed with this, It could be helpful to revisit what Astro decided to do with focus in their transition:persist directive.

Regarding the (Alpine.js?) state: usually, morphing and persisting is exactly meant for persisting state. (see Alpine.js' Morph Plugin or the headline in the Astro docs ("Maintaining State").

I think this is a hard problem that I'd love to discuss in a call between the three of us sometime soon, where we can look at our real-world use cases together and discuss if this can be generalized somehow without silently breaking anyone's implementation.

It might turn out to be the cleanest solution to introduce a new attribute data-swup-persist-focus or the like, to give users the power (and responsibility!) of managing this without a custom hook as I used above. But then again, what should win if a11y-plugin is installed?! 😵‍💫😄 ...as I said, we should talk this through.

@hirasso
Copy link
Member

hirasso commented Jun 19, 2024

As discussed, we are closing this for now. Users can easily implement their own solution using a hook as described above. We should keep in mind to think about merging swup/a11y-plugin into core, to make accessibility a first class citizen in swup's ecosystem.

@hirasso hirasso closed this Jun 19, 2024
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.

None yet

3 participants