Repository navigation
MenuItem decomposition Phase 4 — style-prop remnants #100159
Description
Activity
- added a parent issue
on Sep 2, 2026 - changed the title
[-]MenuItem decomposition Phase 4 — style-prop remnants[/-][+][Due for payment 2026-09-15] MenuItem decomposition Phase 4 — style-prop remnants[/+]on Sep 8, 2026 The solution for this issue has been 🚀 deployed to production 🚀 in version 9.4.73-3 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:
If no regressions arise, payment will be issued on 2026-09-15. 🎊
The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the Contributor+ Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]. If no checklist is needed for this issue, you can click: [no checklist button]
@MelvinBot provide a status update on this issue. Is there more code work needed, or are we just in a waiting-for-payment holding pattern?
Reacted by melvin-botMore code work is needed — this isn't a pure payment hold. #99915 delivered one of the eight items in the Work list (the 13-site
outerWrapperStylecluster). Every other style prop in the inventory is still onmainuntouched.The payment automation has already run ahead of the scope, though: the title carries
[Due for payment 2026-09-15]andPujan92's C+ checklist is still outstanding. So you have two things to reconcile.What's still open, verified at
bc098e0Work item Status outerWrapperStyle× 13✅ Done in #99915 — moved to caller-owned Views, no new variant neededDelete rootWrapperStyle(dead prop)❌ Still declared at MenuItem.tsx:396and threaded to:780Migrate the one innerContainerStylesite, drop the prop❌ Still at IndividualExpenseRulesSectionRevamp.tsx:258titleContainerStyle× 3❌ TripRoomPreview:105,TripDetailsView:160,DynamicReportDetailsPage:928— plus a 4th the inventory missed,WorkspaceReceiptPartnersPage:260titleWrapperStyle× 2❌ DynamicSplitExpensePage:578,DynamicSplitExpenseEditPage:411rightIconWrapperStyle× 2❌ DiscoverSection:98,UpcomingTravelItem:133iconStyles(~15)❌ 25 iconStyles=sites remain, including both target clusters —[styles.ml3, styles.mr2]atBankAccountPurpose:37/CardTypeStep:92and the trip-reservation containerstyle/wrapperStyle/containerStylelong tail❌ Never enumerated. #100585 ( wrapperStyle) was opened and closed empty on Sep 8, so there's intent but no live PRWrite down the escape-hatch policy ❌ Not in the repo — nothing under src/components/MenuItem/orcontributingGuides/records the three optionsExtend the Storybook MenuItemComparisongallery❌ #99915 didn't touch MenuItemComparison.stories.tsxRecommendation
Pay this one out and split the remainder into Phase 4b. The title already has a payment date, so melvin-bot will close it on the 15th regardless — fighting that is more friction than filing a follow-up. This also matches how Phases 1 and 2 were handled: one issue per reviewable PR chunk rather than one issue per phase.
Concretely:
Pujan92posts the C+ checklist so the Sep 15 payment isn't blocked. Nothing else is in the way — the last open item,m-natarajan's Test 3 / Step 6 question, was answered bywar-inon Sep 8, and the PR shipped to production in 9.4.73-3.- Open Phase 4b for the nine remaining items, with the escape-hatch policy write-up as the first deliverable — it gates the rest, and the
outerWrapperStylePR already established the precedent (option 1, move to caller) that the policy should codify.
Reply with
@MelvinBot create the Phase 4b issueand I'll file it against the parent, carrying the verified inventory above.@roryabraham I'm going to take care of the other parts of that issue (enumerated by Melvin) in a future PRs. However, I'd rather wait for #100191 and the next
MenuItemWithTopDescription-related PRs because they'll add more required leavesReacted by Rory Abraham42 remaining items
- changed the title
[-][Due for payment 2026-10-05] MenuItem decomposition Phase 4 — style-prop remnants[/-][+][Due for payment 2026-10-12] MenuItem decomposition Phase 4 — style-prop remnants[/+]on Oct 5, 2026 The solution for this issue has been 🚀 deployed to production 🚀 in version 9.5.1-9 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:
If no regressions arise, payment will be issued on 2026-10-12. 🎊
The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the Contributor+ Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]. If no checklist is needed for this issue, you can click: [no checklist button]
- changed the title
[-][Due for payment 2026-10-12] MenuItem decomposition Phase 4 — style-prop remnants[/-][+]MenuItem decomposition Phase 4 — style-prop remnants[/+]on Oct 6, 2026 Please comment and tag me if/when payment is due, thx
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.5.6-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:
- MenuItem decomposition - remove
containerStyle#103039 - [No QA] MenuItem decomposition - remove unused
rootWrapperStyle#103035 - MenuItem refactor - remove noop
titleWrapperStyleprop #103037 MenuItemdecomposition - description text style #102263- MenuItem decomposition - remove
rightIconWrapperStylefrom legacyMenuItem#103038
- MenuItem decomposition - remove
Phase 4 — style-related remnants
Part of the
MenuItemdecomposition rollout plan. Phase 1 landed in #97339 (MenuItemNavigation/MenuItemAction); Phase 2 is tracked in #99408. This is the style-prop phase.Scope
Most of the remaining call sites differ from a preset by nothing but a style prop —
style,wrapperStyle,outerWrapperStyle,innerContainerStyle,containerStyleand their siblings. Migrate them by deciding, per prop, whether the style can move outsideMenuItemor needs a named variant on the subcomponent that owns that box.The decision this phase has to make first
The new primitives currently expose no style prop at all —
MenuItemRoot.tsx:77-86hardcodes its style array, andMenuItemRow/MenuItemContentdo the same. That's the right default: a genericstylepassthrough on every subcomponent rebuilds the monolith one prop at a time.So before migrating anything, settle the escape-hatch policy. For each style prop, one of:
View(preferred).Per the plan's cross-cutting rule: no preset grows a prop for a minority need.
Verified inventory
Counts are MenuItem-scoped call sites at
3a621ed, excluding the shared wrappers (Phase 5).outerWrapperStyleshouldUseNarrowLayout ? styles.mhn5 : styles.mhn8. A singleMenuItem.Rootvariant covers the whole cluster.iconStyles[styles.ml3, styles.mr2]×4 and the trip-reservation icon container. Rest are one-offs (styles.h7,styles.mr0,styles.wAuto).titleContainerStyleTripDetailsView,TripRoomPreview,DynamicReportDetailsPage— all gap/alignment. →MenuItem.Contentvariant.titleWrapperStyleDynamicSplitExpensePage,DynamicSplitExpenseEditPage— bothstyles.flex1.rightIconWrapperStyleDiscoverSection,UpcomingTravelItem— bothstyles.pl2.innerContainerStyleIndividualExpenseRulesSectionRevamp.tsx:237(styles.gap5). The other 34innerContainerStyle=hits insrcare Modal/Popover, notMenuItem. Migrate the one, then drop the prop.rootWrapperStyleMenuItem.tsx:458and threaded to:845, but no call site passes it. Straight deletion.style/wrapperStyle/containerStyleMenuItemconsumers — rawsrccounts (~358wrapperStyle=, ~173containerStyle=) are dominated by other components. The first PR should enumerate theMenuItem-scoped subset and group it; expect the same "one repeated expression per cluster" shape asouterWrapperStyle.The full
MenuItemstyle surface is declared atMenuItem.tsx:120-138.All 13
outerWrapperStylesites, for reference:ReimbursementAccount/VerifiedBankAccountFlowEntryPoint×5 — e.g.:299ReimbursementAccount/USD/ConnectBankAccount/components/FinishChatCard×3ReimbursementAccount/ConnectedVerifiedBankAccount×2settings/Wallet/InternationalDepositAccount/subPages/AccountFlowEntryPoint×2ReimbursementAccount/NonUSD/Finish×1Work
rootWrapperStyle— dead prop, no consumers.innerContainerStylesite, then remove the prop.MenuItem.Rootbleed/full-width variant and migrate all 13outerWrapperStylesites in one PR — they share one expression, so it's one decision reviewed once.MenuItemwith no visual change. If it can't, add the variant and handle it inside the owning subcomponent.MenuItemComparisongallery with a section per variant introduced — old config API vs. new composition side by side.Out of scope
FocusableMenuItem,HighlightableMenuItem,HighlightableMenuItemWithTopDescription,MenuItemList, and the PopoverMenu v2 rows are Phase 5 — even though 2 of the 15outerWrapperStyle=occurrences and bothadditionalIconStylesoccurrences live in them.titleStyle,descriptionTextStyle,labelStyle,helperTextStyle,errorTextStyle) belong with the label/error/hint surface in Phase 3, unless one blocks a Phase 4 migration.Notes
<MenuItem … />config API stays untouched.Parent: #96202
Issue Owner
Current Issue Owner: @Pujan92