8000 Fix/1.x/776 guides prev next block template prints url fragement twice by ctorgalson · Pull Request #777 · localgovdrupal/localgov_base · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix/1.x/776 guides prev next block template prints url fragement twice #777

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

Conversation

ctorgalson
Copy link
Contributor
@ctorgalson ctorgalson commented May 21, 2025

Closes #776

What does this change?

This makes the following changes:

  • returns the theme's guides-prev-next-block.html.twig template to its pre Prev-Next SDC condition: namely it no longer sets a URL fragment on each of the Previous and Next links (NOTE: localgov_base.theme still does this),
  • slightly rewrites the theme's guides behavior javascript for efficiency

How to test

  1. check out this branch in a running instance of LGD (the demo site, or any LGD site whose child theme has not modified Guides' Prev/Next links will do),
  2. load a Guide Overview and/or Guide Page and verify that its Prev/Next links contain the fragment #lgd-guides__title, and not #lgd-guides__title#lgd-guides__title
  3. click a Prev/Next link and verify that the focused element on the loaded pages is, in fact, #lgd-guides__title

How can we measure success?

This should return the Prev/Next links to a slightly more accessible condition: the fragments in the URLs will refer to elements that exist on the page and so can be focused, aiding keyboard navigation for users.

Have we considered potential risks?

n/a

Images

Before

image

After

image

Accessibility

(The second two items don't apply to this change).

…comments

This task is already performed in localgov_base.theme, *and* in that
context it's not necessary to pre-render the URL (which is more
efficient).
- window.location always includes a hash property, even if it's empty,
- if it's not empty, it always takes the form of an id selector (e.g.
  #foo-bar),
- using context + once prevents this from running multiple times
It's part of a library, so it runs only where it's supposed to 🤦‍♂️.
Copy link
Member
@markconroy markconroy left a comment

Choose a reason for hiding this comment

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

LGTM

@ctorgalson
Copy link
Contributor Author

Just realized while walking the dog this fails to address a minor edge-case. I'll fix it.

- if it exists, it'll be at least '#', but that's not a valid id
  selector
- the length check ensures that we at least have the hash character
  plus one character, thus a valid selector
@ctorgalson
Copy link
Contributor Author

Fixed now.

@finnlewis finnlewis merged commit c36f29b into 1.x May 27, 2025
8 of 17 checks passed
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.

Guides Prev-Next block template prints url fragment twice
3 participants
0