Skip to content

[Studio UI] Fix sort button triggering on any keypress - #4047

Open
idaiv wants to merge 4 commits into
2026.xfrom
fix/sort-button-key-guard
Open

[Studio UI] Fix sort button triggering on any keypress#4047
idaiv wants to merge 4 commits into
2026.xfrom
fix/sort-button-key-guard

Conversation

@idaiv

@idaiv idaiv commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Sort button onKeyUp fired the sort toggle on every keypress, causing unintended sort changes
  • Now restricted to Enter and Space only
  • Fixes WCAG 2.1 2.1.1 (Keyboard)

Test plan

  • Focus a sortable column header's sort button via Tab
  • Press random keys (letters, arrows) — verify sort does NOT change
  • Press Enter — verify sort direction toggles
  • Press Space — verify sort direction toggles

🤖 Generated with Claude Code

Relates to https://github.com/pimcore/product-management/issues/372

The onKeyUp handler called onClick on every key, causing unintended
sort direction changes during normal keyboard interaction. Now only
Enter and Space trigger the sort toggle.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings August 31, 2026 13:18
@idaiv idaiv added the Skip Milestone Check Exempts this PR from the mandatory milestone requirement label Aug 31, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Verdict: Needs changes. The change correctly limits sort activation to Enter and Space at the owning component.

Changes:

  • Filters keyboard-triggered sorting by key.
  • Covers all SortButton consumers without API changes.
  • Missing regression tests; Space can still scroll because cancellation occurs on keyup.
  • No documentation changes are necessary.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +40 to +45
onKeyUp={ (event) => {
if (event.key === 'Enter' || event.key === ' ') {
event.preventDefault()
onClick()
}
} }
Comment on lines +40 to +45
onKeyUp={ (event) => {
if (event.key === 'Enter' || event.key === ' ') {
event.preventDefault()
onClick()
}
} }
idaiv and others added 3 commits August 31, 2026 13:23
preventDefault on keyup is too late to cancel Space's default scroll.
Switch to onKeyDown so Space activation doesn't also scroll the page.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@idaiv
idaiv requested a review from vin0401 September 2, 2026 09:08
@idaiv idaiv added this to the 2026.3.0 milestone Sep 2, 2026
@idaiv
idaiv requested a review from martineiber September 2, 2026 10:27
@idaiv idaiv removed the Skip Milestone Check Exempts this PR from the mandatory milestone requirement label Sep 2, 2026
@idaiv
idaiv removed the request for review from martineiber September 2, 2026 10:35
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.

2 participants