Skip to content

Conversation

@adamjohnson
Copy link
Collaborator

@adamjohnson adamjohnson commented Jan 20, 2026

What I did

  1. Changes to the AT Focus, Combobox, and RovingTabIndex controllers for rh-select.
  2. For further information about these changes, see "Notes to reviewers" below

Testing Instructions

  1. This PR modifies the RTI, Combobox, and ATFocus Controllers. Test any PFE elements that use these controllers via the DP and ensure the functionality matches what's in production.
  2. Once you've verified everything works as expected on the PFE component side, clone the repo to your machine and run an npm run build.
  3. Copy the following files:
    • core/pfe-core/controllers/at-focus-controller.js + .d.ts + .js.map
    • core/pfe-core/controllers/roving-tabindex-controller.js + .d.ts + .js.map
    • core/pfe-core/controllers/combobox-controller.js + .d.ts + .js.map
  4. Navigate to RHDS on your local machine.
  5. Go to node_modules/@patternfly/pfe-core/controllers
  6. Paste in the files from step 3.
  7. type npm run dev in your terminal.
  8. Verify rh-select works as expected.
  9. Verify other components in RHDS that use any of these controllers work as expected
  10. Like and subscribe.

Notes to Reviewers

I'll outline what changed and why for each controller:

AT Focus Controller

Prior to these changes, when pressing the Home key for rh-select, focus incorrectly moves to the last item instead of the first. This happens because the atFocusedItemIndex setter in ATFocusController calculates the search direction incorrectly.

If the first item is non-focusable (e.g., a disabled placeholder), the while loop searches backward from index 0, wraps to the last item, and stops there.

The solution is to modify the direction calculation in the atFocusedItemIndex setter to handle Home/End explicitly. See notes in code for further details.

Combobox Controller

When using a combobox/select component without a text input (e.g., rh-select), clicking the toggle button while the listbox is open does not close the listbox as expected. Instead, the listbox briefly closes and immediately reopens, preventing the native select-like toggle behavior.

The #onFocusoutListbox handler correctly closes the listbox when focus leaves to an external element, but it doesn't account for the case where focus moves to the toggle button itself.

The solution is to modify the #onFocusoutListbox handler to also check if the relatedTarget is the toggle button. If focus moved to the toggle button, skip the automatic close and let the #onClickButton handler manage the toggle behavior.

RovingTabIndex Controller

The RovingTabindexController maintains an internal atFocusedItemIndex that tracks which item should have focus. However, this index is only updated via keyboard navigation, not when an item receives DOM focus programmatically or via mouse click. This causes keyboard navigation to start from the wrong position after mouse interactions.

Eg: Open the a select and click "Item 3". Item 3 is focused. Re-open the select and hit the down arrow.

Expected: Focus moves to "Item 4". Actual: Focus moves to Item 1.

This change adds a persistent focusin event listener in the RovingTabindexController constructor that syncs atFocusedItemIndex when any item in the container receives DOM focus. This keeps our mouse + keyboard focus in sync.

To Do's

  • Changeset (one or three?)
  • Look into / resolve failing tests around <pf-select>

@adamjohnson adamjohnson self-assigned this Jan 20, 2026
@changeset-bot
Copy link

changeset-bot bot commented Jan 20, 2026

⚠️ No Changeset found

Latest commit: 4132c4c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@netlify
Copy link

netlify bot commented Jan 20, 2026

Deploy Preview for patternfly-elements ready!

Name Link
🔨 Latest commit fb0fbd2
😎 Deploy Preview https://deploy-preview-2963--patternfly-elements.netlify.app/

To edit notification comments on pull requests, go to your Netlify site settings.

@github-actions github-actions bot added the AT passed Automated testing has passed label Jan 20, 2026
@github-actions

This comment has been minimized.

@adamjohnson adamjohnson removed the AT passed Automated testing has passed label Jan 20, 2026
@github-actions github-actions bot added the AT passed Automated testing has passed label Jan 20, 2026
@github-actions
Copy link
Contributor

SSR Test Run for fb0fbd2: Report

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

Labels

AT passed Automated testing has passed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants