Fix: Yes and No whisper buttons have different shapes#82872
Fix: Yes and No whisper buttons have different shapes#82872
Conversation
The actionableItemButton style (borderRadius: 6) was being applied unconditionally to all non-primary buttons, causing a visual inconsistency with the primary button's pill shape (borderRadius: 100). This gates the style override to only apply when layout is 'vertical', which is the context where text wrapping necessitates the different border radius. Co-authored-by: Jules <Julesssss@users.noreply.github.com>
|
🚧 @Julesssss has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
1 similar comment
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
Hey @marcochavezf, could you please check these buttons are styled correctly now? thanks |
|
Do we need C+ review here? |
|
@MelvinBot I think we can take out the condition of !item.isPrimary and just use the layout prop check |
Co-authored-by: Pujan Shah <Pujan92@users.noreply.github.com>
|
Good call — removed the |
|
@huult @Julesssss One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
Hey, I'm the autor of the original PR, I think @Expensify/design should to confirm on what condition the buttons should have old shape. The original request came from them. |
|
We've clarified this a few times now: we only want to use the more square looking buttons for phrasal responses. The other buttons should remain unchanged. |
Explanation of Change
PR #82113 moved
innerStyleslogic from caller-provided props intoActionableItemButtonsitself, applyingstyles.actionableItemButton(withborderRadius: 6) to all non-primary buttons. This caused a visual inconsistency in the room mention whisper: the "Yes" button (primary,borderRadius: 100pill) and the "No" button (non-primary,borderRadius: 6rounded rectangle) had different shapes.This fix gates the
actionableItemButtonstyle override onprops.layout === 'vertical', so it only applies in vertical-layout contexts (like Concierge follow-up suggestions that need text wrapping), and not in horizontal-layout contexts (like the Yes/No room mention whisper buttons).Fixed Issues
$ #82867
PROPOSAL: #82867 (comment)
Tests
#heyand send itOffline tests
N/A - This is a visual styling fix that does not involve network requests.
QA Steps
#heyand send itPR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand 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
N/A - styling change, CI will validate
Android: mWeb Chrome
N/A - styling change, CI will validate
iOS: Native
N/A - styling change, CI will validate
iOS: mWeb Safari
N/A - styling change, CI will validate
MacOS: Chrome / Safari
N/A - styling change, CI will validate