-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Interactivity API: Override unfinished navigate
calls
#54201
Conversation
Size Change: +207 B (0%) Total Size: 1.52 MB
ℹ️ View Unchanged
|
Flaky tests detected in 43556a0. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6089995120
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very good test!! It's pretty smart 🙂
My only concern is why should we use url
instead of href
? (href
also contains the hash)
packages/interactivity/src/router.js
Outdated
// Navigate to a new page. | ||
export const navigate = async ( href, options = {} ) => { | ||
const url = cleanUrl( href ); | ||
navigatingTo = url; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the logic for using url
instead of href
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good question. 😄 I think I assumed that, as the url
would be the same for href
with hashes, both navigations would wait for the same Promise so that they would happen in order and there would be no problem.
I missed considering that:
- You could use
force
and request the HTML again. - Both navigations would trigger a render, which is bad.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think it's a bit safer this way. That way, the user will end up with the correct hash in the browser URL (even though we don't support scrolling to the id yet).
Thanks David!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks David! 🙂
What?
Improve
navigate()
to render only the result of the last call when multiple happen simultaneously.Why?
Required for the Query Loop block with enhanced pagination.
store()
API #53740How?
Previous
navigate()
calls are not canceled, so the related HTML is still fetched. However, only the last one executes Preact'srender()
.Testing Instructions