Skip to content

Optionally update :target styles #162

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

Closed
jakearchibald opened this issue Aug 27, 2021 · 8 comments
Closed

Optionally update :target styles #162

jakearchibald opened this issue Aug 27, 2021 · 8 comments
Labels
addition A proposed addition which could be added later without impacting the rest of the API

Comments

@jakearchibald
Copy link

Currently the target element is set as part of scrolling https://html.spec.whatwg.org/multipage/browsing-the-web.html#scroll-to-fragid:target-element-3, so it doesn't happen as part of pushState.

stephband suggested on Twitter that they'd like the target styles to update.

Perhaps the easiest way to handle this is to allow the target element to be get/set via something like document.targetElement.

@stephband
Copy link

stephband commented Aug 27, 2021

This would still allow the :target element to become desynchronised from the #hashref. My problem with :target as it stands it that it becomes desynchronised while navigating programmatically (ie. via the history.pushState()). That is not obvious, or widely known-about, behaviour (and it requires hacking around by pushing a history entry and then calling .back(), which will force it to update).

It would make the behaviour of :target a lot more understandable if it simply reflected the hashref without us having to manually synchronise it. I would much prefer it if that were possible to include as default behaviour for appHistory's navigate(). Or even opt-in behaviour for navigate().

I realise It is not directly related to the API, but it's one of those things that costs several hours to analyse, test and fix after you first discover your assumptions are wrong, and I would like to make future front-end's lives easier.

@domenic
Copy link
Collaborator

domenic commented Nov 4, 2021

So in spec terms, the problem is that the document's target element is only ever set when scrolling to a fragment. It is not updated if you do, e.g., history.pushState(null, "", "#foo").

I can see two ways app history could help with this problem:

  1. Whenever you use an app history method to navigate (appHistory.navigate() or appHistory.reload()... maybe appHistory.back()/forward()/goTo() as well? not sure) we run a new "synchronize :target algorithm", which updates the target element according to the current URL.

  2. Whenever you use event.transitionWhile() inside your navigate handler, we run the "synchronize :target algorithm".

The difference is that (2) would affect all single-page app navs in an app history-using application, including ones that use history.pushState(null, "", "#foo") or history.pushState(null, "", "/new-path-with-no-fragment"). That's a bit magical, but I think I still prefer (2) over (1).

This is related to other discussions of how, when using event.transitionWhile(), we want various magic to happen:

  • Accessibility tech should be told about the navigation, like is done for a cross-document navigation. (Even though AT isn't currently told about history.pushState(null, "", "#foo") navigations.)

  • We reset focus to the body element, like is done for a cross-document navigation. (Even though currently history.pushState(null, "", "#foo") does nothing with focus.)

  • We probably want to do something with scroll restoration, such as delaying it until the promise passed to transitionWhile() is settled.

My current thinking is that some of these (e.g. the focus or scroll restoration behaviors) will have an opt-out, but do the nice magic thing by default. I think synchronizing :target might fall into the same category.

@domenic domenic added this to the Might block v1 milestone Nov 4, 2021
@domenic domenic added the addition A proposed addition which could be added later without impacting the rest of the API label Nov 4, 2021
@bathos
Copy link

bathos commented Nov 5, 2021

We reset focus to the body element, like is done for a cross-document navigation.

Will this be controllable? This would seem to be at odds with patterns like calendar views that keep /year/month in sync as you use the arrow keys or other controls. This behavior would make sense for transitions to "new page" views in an MPA sense, but not all transitions are.

(I appreciate that currently there's a problem where folks don't know to blur when they should, but I can think of a few other places in the app I work on now where this behavior would break the keyboard/AT experience rather than improve it, so I hope it can be overridden where needed if enabled by default.)

Edit I just caught that this was already answered at the end, oops:

My current thinking [...] will have an opt-out

(Hopefully a useful data point anyway re: the opt-out having value.)

@domenic
Copy link
Collaborator

domenic commented Jan 10, 2022

I'm unsure what to do with this issue in terms of defaults. As seen in #197, we're using app history to introduce a number of new defaults for things like accessibility, focus management, and scroll restoration when you opt in using event.transitionWhile(). In those cases there's clear consensus the current setup is bad. But does this issue fall into that category?

It'd be easy to add something in the future like e.transitionWhile(x, { updateTargetPseudo: true }). But if we want the default when using app history to be updating :target, similarly to the other changed defaults in #197, then we need to figure that out before the initial version of app history ships.

I'd love to hear more from web developers as to whether they've run into this problem. Or, alternately, whether they have use cases for keeping :target and location.hash desynchronized, like they can be currently.

@stephband
Copy link

@domenic Thanks. I have put a call out on Twitter https://twitter.com/stephband/status/1480677513626013700.

@stephband
Copy link

Here is a test page that demonstrates problems with :target:

https://stephen.band/target/

@domenic
Copy link
Collaborator

domenic commented Jan 11, 2022

After thinking about this issue more I think tying this to app history is the wrong approach.

Fundamentally @stephband's model (and it's a reasonable one) is that :target should match the current URL fragment.

But that's not what :target is designed as. It's instead designed to match a specific element which is identified at a specific time very early during page load, or after fragment navigation. It doesn't update whenever the DOM changes, and it doesn't update whenever the URL fragment changes. Instead it always a pointer to a specific element, similar to :focus.

We could try to hack things closer to @stephband's preferred model when app history is involved, by essentially updating that element pointer more often. But that will still leave gaps, and it would shut out people who want to use :target for its current use cases on the same page. Instead, the right approach is to introduce a new selector (e.g. :current-url-fragment) for a new use case.

As such, I'll close this issue and momentarily post a new feature suggestion on w3c/csswg-drafts, linking here.

@kizu
Copy link

kizu commented Jan 21, 2023

Just want to bump this — this issue is the one that constantly comes up in my practice, in sometimes it doesn't have any solution or workaround.

Worst case: when a page has an anchor, and its content is dynamic in any way where the element won't exist initially. This would mean that we would be unable to share a link to this page where the :target would work.

The scrolling to the element can be worked around in various ways (most straightforward way — to have a mutation observer that waits for the element with the id matching the anchor to appear), but there is just no way to update the :target without adding a new history entry.

And the workarounds to have when navigating via pushState are far from convenient (replace the content, navigate to the anchor, so the :target would trigger, replaceState with the new URL, replacing the entry with the anchor).

I don't care much if the solution would be adding a new pseudo-class (though I agree that :target should just work, and :target-no-really is just weird), adding something like an opt-in to the pushState/replaceState where it would apply the target to the given anchor (this way the initial load issue could be potentially worked around via calling replaceState), or maybe just adding a method that invalidates the old :target and applies the new one.

But this is a real issue that comes up constantly, and I really hope we would have a solution for this one day.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition A proposed addition which could be added later without impacting the rest of the API
Projects
None yet
Development

No branches or pull requests

5 participants