Repository navigation
Conversation
|
The Pull Request introduced native fingerprint changes against the base commit. Generated by PR labeler 🤖 |
|
0ab3878 to
425ce1c
Compare
4039efe to
c9e2950
Compare
c9e2950 to
cb21945
Compare
|
I found no actionable defects in this PR. Claude review, run |
cb21945 to
da2295d
Compare
| } | ||
| return { | ||
| selected: radio?.props.selected, | ||
| content: radio ? [radio, ...rest, ...icons] : [...icons, ...rest], |
There was a problem hiding this comment.
non-blocking: a disclosure chevron ends up in front of the label
Moving every ItemIcon to the leading side also moves directional affordances that only make sense trailing. On web, the composer's language menu (src/view/com/composer/select-language/PostLanguageSelect.tsx:126-129, which has no .web variant) renders "More languages..." with <Menu.ItemIcon icon={ChevronRightIcon} /> after the text. With this change the right-pointing chevron is drawn before the label (› More languages...), so it no longer reads as "opens more". The old ItemIcon honoured the written order.
Possible fixes: keep trailing-only glyphs like ChevronRight after the text in arrangeItemContent, keep position="right" meaningful on web, or change that call site.
There was a problem hiding this comment.
Fixed in a3e1ade: arrangeItemContent keeps a right chevron trailing, like Android's submenu arrow, so "More languages..." reads as opening more options again.
|
One non-blocking finding, posted inline at
I found no other defects. Claude review, run |
|
I found no defects in this PR that need fixing. These are the parts I checked:
Most CI jobs (Lint, PR Tests, Lockfile) were still running when I looked, so this review doesn't cover their results. Claude review, run |
a3e1ade to
fe6b6a4
Compare
|
I found no defects to act on in this PR. Claude review, run |
fe6b6a4 to
ac3d812
Compare
|
I found no actionable defects in this PR. Claude review, run |
ac3d812 to
d1d9e7e
Compare
|
I found no actionable defects in this PR. Claude review, run |
d1d9e7e to
0a3cca5
Compare
|
No actionable defects found. Claude review, run |
|
I found no actionable defects in this PR. Claude review, run |
0a3cca5 to
12e57c3
Compare
|
I found no actionable defects in this PR. Claude review, run |
12e57c3 to
785c79c
Compare
|
I found no defects that need fixing in this PR. Claude review, run |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ers positional Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
785c79c to
443a9ed
Compare
Findings: None. Worth a close look
Claude review, run |
Part of APP-3734. Stacked on #12296.
Restyles the web dropdown menu (
src/components/Menu/index.web.tsx) to match the design system's menu design (Figma "Single select menu"), so it looks like the Android inline menus from #12271. No call sites change.border_contrast_lowborder, the Figma "Large Drop" shadow (0 16px 24px, 8% black in light, 40% in dark and dim), 160px min widthnegative_500, disabledtext_contrast_lowmd, to balance the 13px labels. Items have 4px less padding on the leading side (12px), so icons sit closer to the edge. Always on the leading side, in the text colour, whatever order they're written in.Itemre-arranges its own children, like the native inline code.ItemIcon'spositionnow does nothing on webprimary_50pill, aprimary_500label, and the mid-weightCheck_Stroke3from #12271. Dark and dim go a shade lighter (primary_100andprimary_600; their palettes are inverted), so the selected state isn't muddy. A radio item's icon trails, and turns primary when selectedLabelText)bg_contrast_50in light,bg_contrast_100in dark and dim. A hovered selected item deepens toprimary_100primary_500ring inside the item, on top of the hover fill. The fill alone was about 1.1:1 on white. Radix also focuses items as the pointer moves, so the ring only shows when the browser treats the focus as keyboard focus (:focus-visible). Arrow keys move between items; Radix keeps Tab inside the menu, as usual for menusShapes
These follow Material 3's segmented menu tokens, like Android:
SegmentedGap), so a hovered item's fill doesn't touch the selected one.On web,
Groupdoesn't wrap its items, so every item in a menu is a DOM sibling. That includes items rendered by custom components, likePostMenuControlItemsin the post…menu. The rules insrc/style.cssmatch[data-menu-item]with:nth-child(1 of …),:nth-last-child(1 of …)and:has(+ [data-menu-label]). So they count across sections and skip hidden items, with no effects or context.The native
InlineOutercounting can't do this, because it can't see inside custom components. Items carrydataSetbecause react-native-web'sPressabledropsclassName. For the same reason, the old.radix-dropdown-item:focusrule has never applied; it's left as is.Itemnow passesdisabledto Radix, so keyboard navigation skips disabled items instead of landing on them.Notes
position="right"onMenu.ItemIconis used at 54+ call sites and now has no effect on web. Removing it could be a follow-up.t.atoms.textis#000000in light, while Figma'sText/defaultis#111822(contrast_975). Android usest.atoms.texttoo, so the design library and the code disagree.ShareMenuItems.web.tsxusesLabelTextas a footnote, so its note is now semibold in the text colour rather than grey.Testing
pnpm typecheck,pnpm lint,pnpm test src/components/Menu:nth-child(… of …)and:has()(Safari 15.4+, Firefox 121+); otherwise items just keep 4px cornersVideo
web-menu.mov
🤖 Generated with Claude Code