Skip to content

Conversation

@retrofox
Copy link
Contributor

@retrofox retrofox commented Apr 3, 2025

Introduces a new <NavigableRegion /> component that creates accessible regions with keyboard navigation support.
This component follows Gutenberg's pattern for creating navigable sections in the admin interface, providing proper ARIA attributes and keyboard interaction capabilities.

The component is flexible, allowing different HTML elements via the as prop while maintaining consistent accessibility features across the interface.

Related to ARC-27

Proposed Changes

  • [Site Admin]: Add NavigableRegion component

Why are these changes being made?

Testing Instructions

  • Visual review will be enough for now. In the ARC-25 follow-up, we will put this component into practice.

Pre-merge Checklist

  • Has the general commit checklist been followed? (PCYsg-hS-p2)
  • Have you written new tests for your changes?
  • Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
  • Have you checked for TypeScript, React or other console errors?
  • Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
  • Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
    • For UI changes, have we tested the change in various languages (for example, ES, PT, FR, or DE)? The length of text and words vary significantly between languages.
  • For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-aUh-p2)?

@retrofox retrofox self-assigned this Apr 3, 2025
@matticbot
Copy link
Contributor

matticbot commented Apr 3, 2025

@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

@retrofox retrofox requested review from ciampo, mirka and tyxla April 4, 2025 10:38
@matticbot matticbot added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Apr 4, 2025
@tyxla
Copy link
Member

tyxla commented Apr 4, 2025

Ah, bummer we're duplicating yet another existing component. I wish we could just use the one from @wordpress/interface.

Copy link
Contributor Author

retrofox commented Apr 4, 2025

AFAIK, it's a private component

@tyxla
Copy link
Member

tyxla commented Apr 4, 2025

Yep, unfortunately. I have a special opinion about components that we need to use externally but we decided to keep private.

I'll defer to @mirka / @ciampo to make the call here.

Copy link
Contributor Author

retrofox commented Apr 4, 2025

The plan is to get everything ready so we can switch to the core one as soon as it becomes public

@ciampo
Copy link
Contributor

ciampo commented Apr 4, 2025

I don't see an alternative, given that the component is not exported from @wordpress/interface, unfortunately.

Although I'm slightly confused by this sentence:

The plan is to get everything ready so we can switch to the core one as soon as it becomes public

I was under the impression that the goal for the @automattic/site-admin package was "visual consistency" with Gutenberg, and that maintaining compatibility with the APIs of the original Gutenberg package (and tweaking code as little as possible) was NOT a goal, like Riad clarified in this comment.

If that's the case, switching to core as soon as it becomes public may not be possible as our version may have diverged signifcantly.

@retrofox retrofox force-pushed the update/add-page-component branch from 65761e1 to 5532b9d Compare April 4, 2025 17:49
@retrofox
Copy link
Contributor Author

retrofox commented Apr 4, 2025

I don't see an alternative, given that the component is not exported from @wordpress/interface, unfortunately.

Although I'm slightly confused by this sentence:

The plan is to get everything ready so we can switch to the core one as soon as it becomes public

I was under the impression that the goal for the @automattic/site-admin package was "visual consistency" with Gutenberg, and that maintaining compatibility with the APIs of the original Gutenberg package (and tweaking code as little as possible) was NOT a goal, like Riad clarified in this comment.

If that's the case, switching to core as soon as it becomes public may not be possible as our version may have diverged signifcantly.

Yes, that makes sense. But if, for any reason, core exposes resources that are copies of them — like the router package, for instance — we can switch to those instead.

@retrofox retrofox force-pushed the update/add-page-component branch from 5532b9d to e6dd1ab Compare April 8, 2025 07:35
@retrofox retrofox requested a review from a team April 8, 2025 08:26
Copy link
Contributor

@ciampo ciampo left a comment

Choose a reason for hiding this comment

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

Pre-approving, given that I won't be very responsive for the rest of the week.

Feel free to merge after addressing #102361 (comment)

@retrofox retrofox force-pushed the update/add-page-component branch from d69b732 to 93cf99b Compare April 8, 2025 16:46
…index.tsx

Co-authored-by: Marco Ciampini <marco.ciampini@automattic.com>
@retrofox retrofox force-pushed the update/add-page-component branch from f5b65e3 to 052d87e Compare April 8, 2025 18:27
@retrofox retrofox merged commit 8bb831c into trunk Apr 9, 2025
14 checks passed
@retrofox retrofox deleted the update/add-page-component branch April 9, 2025 06:51
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Apr 9, 2025
JessBoctor pushed a commit to JessBoctor/wp-calypso that referenced this pull request Apr 15, 2025
* Introduce first approach of NavigableRegion cmp
Create new component for accessible keyboard navigation regions with ARIA support

* improve NavigableRegion component types

* add JSDoc to NavigableRegion component

* changelog

* reorganize interface cmps - expose NavigableRegion cpm

* remove component className
This className is not used for now. We can add it in follow-ups if it's required.

* Update NavigableRegion export to named export

* fix changelog tracks

* Update packages/site-admin/src/interface/components/navigable-region/index.tsx

Co-authored-by: Marco Ciampini <marco.ciampini@automattic.com>

---------

Co-authored-by: Marco Ciampini <marco.ciampini@automattic.com>
JessBoctor pushed a commit to JessBoctor/wp-calypso that referenced this pull request Apr 17, 2025
* Introduce first approach of NavigableRegion cmp
Create new component for accessible keyboard navigation regions with ARIA support

* improve NavigableRegion component types

* add JSDoc to NavigableRegion component

* changelog

* reorganize interface cmps - expose NavigableRegion cpm

* remove component className
This className is not used for now. We can add it in follow-ups if it's required.

* Update NavigableRegion export to named export

* fix changelog tracks

* Update packages/site-admin/src/interface/components/navigable-region/index.tsx

Co-authored-by: Marco Ciampini <marco.ciampini@automattic.com>

---------

Co-authored-by: Marco Ciampini <marco.ciampini@automattic.com>
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.

5 participants