Skip to content

USWDS - Core: Replace receptor library "keymap" with custom implementation#6174

Merged
heymatthenry merged 7 commits into
uswds:developfrom
aduth:aduth-rm-receptor-keymap
Jun 9, 2025
Merged

USWDS - Core: Replace receptor library "keymap" with custom implementation#6174
heymatthenry merged 7 commits into
uswds:developfrom
aduth:aduth-rm-receptor-keymap

Conversation

@aduth

@aduth aduth commented Nov 1, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Reduced the JavaScript bundle size. By optimizing the implementation of core component code, overall JavaScript sizes are reduced.

Breaking change

This is not a breaking change.

Related pull requests

Split from #5793, to simplify review.

Includes additional inline code comments based on review feedback from that pull request.

Preview link

N/A

Problem statement

See #5793

Solution

Replace use of receptor/keymap with equivalent custom implementation.

Testing and review

Verify unit tests pass:

npx gulp unitTests

For user acceptance testing, Modal is a good component to test for regressions, since its focus trap includes both simple key mapping (Tab) as well as modifier combinations (Shift+Tab).

  1. Go to http://localhost:6006/?path=/story/components-modal--default
  2. Click "Open default modal"
  3. Press Tab or Shift+Tab
  4. Observe that modal focus wraps as expected in focus trap

@aduth aduth mentioned this pull request Nov 1, 2024
Closed
@mejiaj mejiaj added this to the USWDS release: January 2025 milestone Jan 3, 2025
@mejiaj mejiaj mentioned this pull request Jan 7, 2025
Closed
mahoneycm
mahoneycm suggested changes Jan 8, 2025
View reviewed changes

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

This seems to be working great @aduth ! The simple replacement in each of the component JS files is a good sign.

I left a few comments for added clarity. Some of the unit test descriptions were confusing to me. I added some suggestions but let me know if I'm off base. Hopefully we can improve their readability a bit so users can understand easily.

Open to other suggestions as well!

Comment thread packages/uswds-core/src/js/utils/keymap.js
Comment thread packages/uswds-core/src/js/utils/keymap.js
Comment thread packages/uswds-core/src/js/utils/test/keymap.spec.js
Comment thread packages/uswds-core/src/js/utils/test/keymap.spec.js Outdated
Comment thread packages/uswds-core/src/js/utils/test/keymap.spec.js Outdated
Comment thread packages/uswds-core/src/js/utils/test/keymap.spec.js Outdated
Comment thread packages/uswds-core/src/js/utils/test/keymap.spec.js Outdated
Comment thread packages/uswds-core/src/js/utils/test/keymap.spec.js Outdated
Comment thread packages/uswds-core/src/js/utils/test/keymap.spec.js Outdated
@aduth aduth requested a review from a team as a code owner January 10, 2025 19:48
@aduth aduth force-pushed the aduth-rm-receptor-keymap branch from 4167c08 to eebd3f8 Compare January 10, 2025 19:59
@mejiaj mejiaj changed the base branch from develop to replace-receptor January 14, 2025 14:12
@mejiaj mejiaj requested a review from mahoneycm February 4, 2025 16:36
@mejiaj

mejiaj commented Feb 4, 2025

Copy link
Copy Markdown
Contributor

@mahoneycm re-requesting review to confirm your comments have been addressed.

@aduth aduth force-pushed the aduth-rm-receptor-keymap branch from eebd3f8 to 4539fd0 Compare February 4, 2025 16:42
@aduth aduth changed the base branch from replace-receptor to develop February 4, 2025 16:43
@mahoneycm

mahoneycm commented Feb 5, 2025

Copy link
Copy Markdown
Contributor

@aduth I left one more suggestion for the direction of the test descriptions! Apologies for not getting back to you sooner.

Thank you for your continued support and input 🙏

@aduth
Clarify test descriptions
0b72172 
Co-authored-by: Charlie Mahoney <charlie.mahoney@bixal.com>
@aduth

aduth commented Feb 6, 2025

Copy link
Copy Markdown
Contributor Author

@mahoneycm Thanks for the re-review. I applied the latest suggestions from the review threads. Let me know if there are others you'd still like to see updated from the unresolved threads.

@dct2023

dct2023 commented Feb 7, 2025

Copy link
Copy Markdown

@mlloydbixal Adding you as a reviewer for this to get us unblocked on #6291

mahoneycm
mahoneycm suggested changes Feb 10, 2025
View reviewed changes

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

@aduth Ok last batch of suggestions to make the existing tests match the new convention and to remove a duplicate test for unexpected modifiers in keypress!

After this we should be good to go

Comment thread packages/uswds-core/src/js/utils/test/keymap.spec.js Outdated
Comment thread packages/uswds-core/src/js/utils/test/keymap.spec.js Outdated
Comment thread packages/uswds-core/src/js/utils/test/keymap.spec.js Outdated
Comment thread packages/uswds-core/src/js/utils/test/keymap.spec.js Outdated
aduth and others added 2 commits February 10, 2025 11:52
@aduth

aduth commented Feb 10, 2025

Copy link
Copy Markdown
Contributor Author

@aduth Ok last batch of suggestions to make the existing tests match the new convention and to remove a duplicate test for unexpected modifiers in keypress!

Thanks for the review. Pushed updates in d0840da and a0ab5a1 based on your feedback.

mahoneycm
mahoneycm approved these changes Feb 11, 2025
View reviewed changes

@mahoneycm mahoneycm 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! Thanks so much @aduth 👍

@mahoneycm mahoneycm requested review from amyleadem, mandylloyd and mejiaj and removed request for a team February 11, 2025 15:41
@heymatthenry heymatthenry mentioned this pull request Jun 9, 2025
Merged
@heymatthenry heymatthenry merged commit cdd9212 into uswds:develop Jun 9, 2025
3 checks passed
@github-project-automation github-project-automation Bot moved this from In progress to Fed Final Review in USWDS Core Project Data Jun 9, 2025
@heymatthenry heymatthenry mentioned this pull request Jun 10, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@heymatthenry heymatthenry heymatthenry approved these changes
@mejiaj mejiaj Awaiting requested review from mejiaj
@amyleadem amyleadem Awaiting requested review from amyleadem
+2 more reviewers
@mahoneycm mahoneycm mahoneycm approved these changes
@mandylloyd mandylloyd mandylloyd approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@mandylloyd mandylloyd

Labels

None yet

Projects

USWDS Core Project Data
Archived in project

Milestone

USWDS release: February 2025

Development

Successfully merging this pull request may close these issues.

6 participants

@aduth @mejiaj @mahoneycm @dct2023 @heymatthenry @mandylloyd