fix: Card details page title and toggle button spacing - #91115
marcochavezf merged 6 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adjusts the layout of ToggleSettingOptionRow (used across multiple settings screens, including the personal card details header menu) to improve spacing between the title/subtitle content and the trailing switch—especially in longer translations where wrapping occurs.
Changes:
- Increased top spacing for subtitles rendered below the switch (
mt1→mt3). - Added right margin to the default title container to create a consistent gap from the switch (
mr3). - Removed a conditional fixed-height constraint on the main row container when subtitles are placed below the switch.
Comments suppressed due to low confidence (2)
src/pages/workspace/workflows/ToggleSettingsOptionRow.tsx:199
- The new right margin (
styles.mr3) is only applied in the defaulttitlerendering path. WhencustomTitleis provided, there’s no equivalent spacing, so custom titles can still end up too close to the switch. Consider moving the margin/spacing to a shared container so it applies to bothtitleandcustomTitle.
{customTitle ?? (
<View style={[styles.flexColumn, styles.flex1, styles.mr3]}>
<Text
src/pages/workspace/workflows/ToggleSettingsOptionRow.tsx:239
- Removing the conditional
styles.h10means the row can shrink to the intrinsic height of its contents (e.g. ~28px for the switch), reducing the tappable area for the row whenshouldPlaceSubtitleBelowSwitchis true. If the fixed height was causing wrapping/clipping issues, consider using aminHeight(e.g.styles.mnh40) instead of removing the height constraint entirely.
<View style={[styles.flexRow, styles.alignItemsCenter, styles.justifyContentBetween]}>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@samranahm Could you merge main? |
…-details-page-style
|
@truph01 All yours, please take a look. |
Reviewer Checklist
Screenshots/Videos |
|
@samranahm Do you think we should revert all changes related to vertical style since it is out of scope? |
|
@truph01 we can absolutely do that but then this way there'd be little vertical spacing between title and subtitle when title wraps to second line. Here's the vertical space when title on single line
Title wraps to second line
|
|
@samranahm Could you ask the design team to confirm about the change we made in vertical spacing? |
|
@Expensify/design We have added vertical spacing on the "Card details" page between the "Mark transactions as reimbursable" title and the subtitle below. Problem: When the title remains on a single line, we get the desired vertical spacing we needed. However, now we’ve introduced title wrapping, the title and subtitle keep their positions intact, which leads to a very narrow gap between the title and subtitle. Can you please take a look at this #91115 (comment) and confirm the vertical spacing? |
|
@dubielzyk-expensify Can you see the above comment please. |
|
The name and toggle are still very close to me. I'd add 8px or so padding. Curious if @Expensify/design agrees though. |
|
Yeah I agree, I think there should be like 24px of visible gap between the elements on each of those rows. It just depends how much vertical padding each row is already wrapped with.... typically we do 12px of vertical padding on each row and then 0 gap between them, which gives us the 24px effect. |
|
Thanks for the feedback! I've updated the gap between from |
|
@marcochavezf Gentle bump. |
|
🚧 @marcochavezf has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/marcochavezf in version: 9.4.10-0 🚀
Bundle Size Analysis (Sentry): |
Help site review — No changes requiredI reviewed the changes in this PR against Expensify's help site files under Conclusion: No help site documentation updates are required. Reasoning: This PR is a purely visual/styling fix. The only file changed is
There are no changes to:
Help site articles document product behavior and user-facing copy/workflows. Since this PR only adjusts pixel-level spacing to fix a layout overlap, there is nothing in the help site that needs to be updated, and no draft docs PR has been created. @samranahm, no help site PR was created because this change is styling-only with no user-facing or behavioral impact. If you believe a docs update is needed for related behavior I may have missed, let me know and I'll take another look. |
|
🚀 Deployed to staging by https://github.com/marcochavezf in version: 9.4.12-0 🚀
Bundle Size Analysis (Sentry): |
|
🤖 No help site changes required. I reviewed the changes in this PR. The only modified file is
These are CSS/layout-only adjustments. No user-facing copy, feature names, settings labels, tab names, buttons, or behaviors were added or changed. Expensify's help site files under @samranahm, since this change doesn't affect any documented behavior, there's no linked help site PR to review. If you believe a specific article is impacted (e.g. a screenshot that should be refreshed), let me know which one and I'll create the draft PR. |
|
🚀 Deployed to staging by https://github.com/marcochavezf in version: 9.4.14-0 🚀
Bundle Size Analysis (Sentry): |
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.4.14-1 🚀
|








Explanation of Change
Fixed Issues
$ #88234
PROPOSAL: #88234 (comment)
Tests
Offline tests
QA Steps
Same as test
// TODO: These must be filled out, or the issue title must include "[No QA]."
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.native.mp4
Android: mWeb Chrome
Android.mWeb.Chrome.mp4
iOS: Native
IOS.Native.mp4
iOS: mWeb Safari
MacOS: Chrome / Safari
macOS.Chrome.mp4
Need help on this PR? Tag
@codesmithwith what you need.