Repository navigation
Remove legacy Button component, clean seatbelt - #100000
Conversation
…omplete
Deletes the old src/components/Button/index.tsx, its story, and its unit
test, plus stale jest.mock('@components/Button') mocks in tests whose
components under test no longer import it.
Guccio163
changed the title
|
|
|
|
Don't forget to remove the ESLint rule too |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
…-fork into Guccio163/button/cleanup
isDeployChecklistLocked and proposalPoliceComment index.js had drifted from main's regenerated bundle output; resync removes unrelated noise from this branch's diff.
getButtonRole always returned the same constant regardless of its isNested arg on both platform variants, so the indirection was dead weight. Inlined the constant at all 15 call sites and removed the now-unused helper and its type.
…onKeyboardShortcut It was only used by ButtonKeyboardShortcut, so move it out of the legacy Button/ folder into a ButtonKeyboardShortcut/ subfolder under ButtonComposed/primitives, alongside the component it serves. Also swap the unsafe HTMLInputElement cast for instanceof narrowing, matching the pattern already used elsewhere (e.g. NavigationFocusReturn), so the moved file keeps a clean eslint-seatbelt baseline instead of inheriting a stale grandfathered violation tied to the old path.
…utton Matches the instanceof pattern already used elsewhere in the codebase (e.g. NavigationFocusReturn) instead of asserting to HTMLElement blindly.
Mocks, imports, and describe/comment text still pointed at the old @components/ButtonComposed path, breaking module resolution now that the composed Button was renamed to Button.
Stale docstrings and a comment still named the old ButtonComposed path; updated them to Button. Also dropped the orphaned eslint-seatbelt row for the deleted ButtonComposed/Button.tsx (local runs are read-only by default so the tool never pruned it itself).
LinkButtonProps, ButtonIconProps, ButtonEventsProps, ButtonBehaviorProps, ButtonStyleProps, and BaseButtonProps were exported but never imported anywhere outside their own file. The ButtonComposed -> Button rename moved this pre-existing knip finding to a new path, which the CI knip-compare check treats as a new violation regardless of the matching resolved one. Dropping the export (the types are still used internally) removes the finding outright instead of just relocating it.
…ibe label Old name/label were leftovers from the ButtonComposed -> Button rename; the collision with the legacy Button test no longer exists since that file was removed.
|
Hey! I see that you made changes to our Form component. Make sure to update the docs in FORMS.md accordingly. Cheers! |
|
@ZhenjaHorbach @mountiny ready ✅ |
|
Let's merge this! |
…-fork into Guccio163/button/cleanup
|
@Guccio163 |
…-fork into Guccio163/button/cleanup
…-fork into Guccio163/button/cleanup
|
Conflicts fixed, waiting for checks to pass |
|
@Guccio163 |
|
I hope we merge this one soon then 😆 |
…-fork into Guccio163/button/cleanup
|
@mountiny |
…-fork into Guccio163/button/cleanup
mountiny
left a comment
mountiny
left a comment
There was a problem hiding this comment.
Thanks for your patience, sorry for the delay
|
🚧 mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
|
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.4.76-0 🚀
|
|
🤖 No help site changes are required for this PR. I did not create a docs PR. Why: This is an internal component refactor with no user-visible change. Across all 398 changed files, nothing touches What I checkedChange surface: 398 files, all under The three changes that could plausibly have been user-facing, and why they aren't:
Button labels are unchanged. Every label in the touched pages still resolves through the same Help articles covering these flows — account security and Copilot setup — describe what the buttons do, not how the component is built, so they stay accurate as written. Since no help site changes are needed, there is no docs PR to review, and nothing to label @Guccio163, please confirm you agree that no help site update is needed here. If you'd rather I create the docs PR anyway, say so and I'll open it as a draft assigned to you. view run · no recording available |
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.4.76-1 🚀
Bundle Size Analysis (Sentry): |
8 participants
This PR finishes the legacy Button to composed Button migration.
Explanation of Change
Fixed Issues
$ #95180
PROPOSAL:
Tests
Most of this PR's changes are NoQA; beside cleanup, the ButtonDisabledWhenOffline was implemented in place of Button's sharing the isDisabled={isOffline} behaviour; Verify offline-disabled buttons (e.g. Workspace Member Details, Add Delegate, Lock Account) show as disabled while offline.
Offline tests
QA Steps
Same as tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Screen.Recording.2026-09-03.at.16.22.08.mov