Skip to content

iAPI Router: Prioritize custom click event handlers when full-page navigation is enabled #70566

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 2, 2025
Merged

iAPI Router: Prioritize custom click event handlers when full-page navigation is enabled #70566

merged 1 commit into from
Jul 2, 2025

Conversation

DAreRodz
Copy link
Contributor

What?

Gives priority to custom click event handlers, typically defined with data-wp-on--click directives, over the global click event handler that controls all site links added by the @wordpress/interactivity-router/full-page module.

This fixes the click logic of the Query Pagination blocks.

Why?

Specific behavior defined in blocks should take precedence over the more general full-page behavior. This block-specific behavior, added to a link, would typically handle navigation.

How?

The useCapture value of true is removed. That would make any event handler calling event.preventDefault() to effectively prevent the event from being captured by the full-page event listener.

Testing Instructions

  1. In a site with the Interactivity API: Full-page client-side navigation experiment enabled, visit a page with a Query block with the Force page reload setting disabled.
  2. Check out trunk
  3. Click on the Query Pagination links
  4. Navigation happens without scrolling to the top of the page.
  5. Check out this branch
  6. Click on the Query Pagination links
  7. Navigation should happen, and the browser scrolls to the top of the page. This is the common behavior when the full-page client-side navigation experiment is disabled.

@DAreRodz DAreRodz added [Type] Enhancement A suggestion for improvement. [Feature] Interactivity API API to add frontend interactivity to blocks. [Package] Interactivity Router /packages/interactivity-router labels Jun 30, 2025
@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 30, 2025 •
edited
Loading

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: DAreRodz <darerodz@git.wordpress.org>
Co-authored-by: SantosGuillamot <santosguillamot@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@DAreRodz DAreRodz mentioned this pull request Jun 30, 2025
Closed
SantosGuillamot
SantosGuillamot approved these changes Jun 30, 2025
View reviewed changes
Copy link
Contributor

@SantosGuillamot SantosGuillamot left a comment

Choose a reason for hiding this comment

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

LGTM 🙂 Seems to work as described.

While testing it, I noticed that it doesn't scroll to the top if there are no links in the query loop (which is the case in the default layout). I assume that's a different discussion, but do you think that should be addressed in other pull request? Or is it expected?

@DAreRodz
Copy link
Contributor Author

DAreRodz commented Jul 2, 2025

Yes, @SantosGuillamot, we would need to address that issue later on.

I think the navigate action from @wordpress/interactivity-router should allow some sort of scrolling behavior control, maybe with focus control as well. In fact, it currently scrolls when navigating to links with hashes, so why not do the same for regular links? 🤷

gutenberg/packages/interactivity-router/src/index.ts

Lines 392 to 396 in 3fd77b3

// Scroll to the anchor if exits in the link.
const { hash } = new URL( href, window.location.href );
if ( hash ) {
document.querySelector( hash )?.scrollIntoView();
}

Also, I'd love to hear from @luisherranz on this matter. 🙂

@DAreRodz DAreRodz merged commit d70798b into trunk Jul 2, 2025
70 checks passed
@DAreRodz DAreRodz deleted the fix/iapi-router-full-page-no-capture-click branch July 2, 2025 07:23
@github-actions github-actions bot added this to the Gutenberg 21.2 milestone Jul 2, 2025
@luisherranz
Copy link
Member

luisherranz commented Jul 2, 2025 •
edited
Loading

it doesn't scroll to the top if there are no links in the query loop

What do you mean by "if there are no links in the query loop"?

why not do the same for regular links?

In my opinion, we should scroll to the top and move the focus to the first focusable element on the page only on regular links of full-page client-side navigation.

The scroll to top and focus control should happen here:

gutenberg/packages/interactivity-router/src/full-page.ts

Lines 23 to 31 in 1d0c43d

// Navigate on click.
document.addEventListener( 'click', async ( event ) => {
const ref = ( event.target as Element ).closest( 'a' );
if ( isValidLink( ref ) && isValidEvent( event ) ) {
event.preventDefault();
const { actions } = await import( '@wordpress/interactivity-router' );
actions.navigate( ref.href );
}
} );

With region-based client-side navigation, things are different. I think whoever calls actions.navigate should be responsible for handling scrolling and updating the focus according to their specific use case.

By default, I don't think scroll should happen in region-based client-side navigation because it's intended to replace content blocks within a page rather than the entire page itself. Typically, users will prefer to keep their scroll position and context during navigation.

Focus in region-based client-side navigation is also tricky because it depends on the use case. Usually, you'll want to set focus somewhere within the region that's been updated. So it should also be the responsibility of whoever is calling actions.navigate.

@SantosGuillamot
Copy link
Contributor

What do you mean by "if there are no links in the query loop"?

From my testing and what I understand from this code, it only focuses and scrolls to the top when there is a link. For example, if I have a Post Title and a Post Image inside the Post Template, it only scrolls to the top when the Post Title has the "Make title a link" setting enabled.

@luisherranz
Copy link
Member

luisherranz commented Jul 3, 2025 •
edited
Loading

Ah, ok, I get it now. Thanks.

It's not actually scrolling to the top. It's scrolling to that link because of the focus. Maybe we should change it to any focusable element, not just links. Although that code already received an accessibility review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SantosGuillamot SantosGuillamot SantosGuillamot approved these changes

@luisherranz luisherranz Awaiting requested review from luisherranz

Assignees
No one assigned
Labels
[Feature] Interactivity API API to add frontend interactivity to blocks. [Package] Interactivity Router /packages/interactivity-router [Type] Enhancement A suggestion for improvement.
Projects
None yet
Milestone
Gutenberg 21.2
Development

Successfully merging this pull request may close these issues.
3 participants
@DAreRodz @luisherranz @SantosGuillamot