Skip to content

Add a go to first/last menu-item helper method in the Menu class - #21690

Merged
Snuffleupagus merged 1 commit into
mozilla:masterfrom
Snuffleupagus:Menu-#goToInitial
Aug 4, 2026
Merged

Snuffleupagus merged 1 commit into
mozilla:masterfrom
Snuffleupagus:Menu-#goToInitial

Conversation

@Snuffleupagus

Copy link
Copy Markdown
Collaborator

This fixes a bug when using the Home and End keyboard shortcuts to navigate through a Menu instance. These two buttons didn't update the #lastIndex field, which means that e.g. a following ArrowDown or ArrowUp press could make focus "jump" to an unexpected menu-item.

Also, the helper method reduces a little bit of code duplication in the event handlers.

@Snuffleupagus Snuffleupagus added viewer accessibility release-blocker Blocker for the upcoming release labels Aug 2, 2026
@Snuffleupagus Snuffleupagus changed the title Add a go to first/last menu-item helper method in Menu class Add a go to first/last menu-item helper method in the Menu class Aug 2, 2026
@codecov-commenter

codecov-commenter commented Aug 2, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.02%. Comparing base (b4ba666) to head (fc610d3).

Files with missing lines Patch % Lines
web/menu.js 66.66% 3 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master   #21690   +/-   ##
=======================================
  Coverage   90.01%   90.02%           
=======================================
  Files         264      264           
  Lines       66893    66894    +1     
=======================================
+ Hits        60214    60219    +5     
+ Misses       6679     6675    -4     
Flag Coverage Δ
browsertest 66.51% <ø> (-0.03%) ⬇️
integrationtest 69.36% <66.66%> (+0.02%) ⬆️
unittest 57.89% <ø> (+<0.01%) ⬆️
unittestcli 56.62% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread web/menu.js Outdated
Comment thread web/menu.js Outdated

@calixteman calixteman 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.

LGTM. It'd be nice to have an integration test too, but if you don't have time to do one, please tell me and I'll do it myself in a follow-up.
Thank you.

This fixes a bug when using the <kbd>Home</kbd> and <kbd>End</kbd> keyboard shortcuts to navigate through a `Menu` instance. These two buttons didn't update the `#lastIndex` field, which means that e.g. a following <kbd>ArrowDown</kbd> or <kbd>ArrowUp</kbd> press could make focus "jump" to an unexpected menu-item.

Also, the helper method reduces a little bit of code duplication in the event handlers.
@Snuffleupagus

Copy link
Copy Markdown
Collaborator Author

It'd be nice to have an integration test too, but if you don't have time to do one, please tell me and I'll do it myself in a follow-up.

It would be great if you could do that in a follow-up, thank you!

@Snuffleupagus
Snuffleupagus merged commit 72a76e5 into mozilla:master Aug 4, 2026
17 checks passed
@Snuffleupagus
Snuffleupagus deleted the Menu-#goToInitial branch August 4, 2026 08:20
calixteman added a commit that referenced this pull request Aug 4, 2026
Follow-up to PR #21690: these tests check that pressing Home/End
correctly updates the last focused menu-item index, so that a following
ArrowUp/ArrowDown press doesn't move focus to an unexpected menu-item.

This branch was previously deployed

1 inactive deployment
code-coverage — fc610d3e Deployed Aug 3, 2026 by Snuffleupagus via Test (22) #16601
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

accessibility release-blocker Blocker for the upcoming release viewer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants