-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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: router with region-based client-side navigation #53733
Interactivity API: router with region-based client-side navigation #53733
Conversation
Size Change: +413 B (0%) Total Size: 1.51 MB
ℹ️ View Unchanged
|
I think it's confusing to use If What do you think, @DAreRodz? |
Agree, and I think those names are easy to understand as well, so you have my +1 👍. |
Let's go with |
@luisherranz, I assume we need to add the |
No, let's keep the navigate logic inside actions instead of directives for now. |
I've just noticed a problem while working on Therefore, I think this should be an option of I also noticed that we are doing the What do you think? Do you see any problem or inconvenience? |
Thanks! I did see that right after I posted 🤦🏼♂️ Global state makes a lot of sense! |
I am not following this PR for now as it looks to still be under dev but please note the accessibility concerns around using JS this way. Dynamically rendering parts of the page comes with huge implications for people who can't see changes happening if not properly communicated. With great power comes great responsibility. This is still a problem in Gutenberg today so I am super hesitant to make this available to the front-end. Please ping me when this is ready for proper review. Thanks. |
Hi @alexstine. Yes, we're aware of the accessibility challenges, and we have plans to work on them. At this moment, we're only experimenting with this here and here, so my plan is to address the accessibility in those PRs. I was planning to ping you once that initial accessibility work was done so you can review it 🙂 Once we learn more, we can find ways to include accessibility on the underlying APIs to ensure people building on top of this make things right (the Interactivity API is still experimental so nobody should be using it in production yet). |
Hey @ajgagnon, I think we're going to add it manually first. Once we learn more, we can look into ways to make it globally accessible. Same for changing the scroll position and accessibility. So for now, we'll use something like this (simplified): store({
actions: {
core: {
query: {
navigate: async ({ ref, context }) => {
context.core.query.isLoading = true;
speak(state.core.query.loadingPageMessage);
await navigate(ref.href);
context.core.query.isLoading = false;
speak(state.core.query.pageLoadedMessage);
const firstFocusableItem = getFirstFocusableItem(ref);
firstFocusableItem.focus();
firstFocusableItem.scrollIntoView();
},
},
},
},
}); |
9bac437
to
79ecdbb
Compare
…ide navigation (#53853) * Add failing test * Fix the test * Add changelog * Fix lint error
* Add failing test * Fix test using key * Replace key with data-wp-key * Refactor test a bit * Add changelog * Add docs * Remove unnecessary paragraph * Fix lint error
…vigation (#53876) * Add failing test * Fix the test
Let's make the |
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.
The tests are great, David!
I can't approve because it's my own PR, but consider them approved 🙂
Docs for the region-based router directives still need to be included. They'll be added in a subsequent PR. |
What?
This PR adds support for region-based client-side navigation.
It works with a new directive called
data-wp-navigation-id
which must be unique. Once those ids are in place, you can usenavigate
(exported for now in@wordpress/interactivity
) to navigate to another page. Only the regions marked withdata-wp-navigation-id
will be replaced.Why?
Because with the Interactivity API, we can replace regions of the DOM with server-generated content while maintaining the UI state.
Use cases include:
How?
Based on @DAreRodz's Woo PR: woocommerce/woocommerce-blocks#10200
Apart from that, I added new options to
navigate
andprefetch
:force
: whentrue
, it bypasses the client page cache.html
: when it's a string, it doesn't fetch the URL and uses that HTML insteadreplace
: when it'strue
, it useshistory.replaceState
instead ofhistory.pushState
.Tasks
We decided to leave these tasks for a following PR: