-
Notifications
You must be signed in to change notification settings - Fork 3.2k
[CP Staging] Fix wrong align of TextInput when displaying currency/icon #55711
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[CP Staging] Fix wrong align of TextInput when displaying currency/icon #55711
Conversation
Hey! I see that you made changes to our Form component. Make sure to update the docs in FORMS.md accordingly. Cheers! |
Chrome video looks good, curious how it looks on other platforms too :) |
@shawnborton this is a bugfix for a regression, and the original bug did not appear on other platforms, so nothing changed there :) |
Ah okay cool, good to know! |
Reviewer Checklist
Screenshots/VideosAndroid: NativeiOS: mWeb SafariMacOS: Chrome / SafariScreen.Recording.2025-01-24.at.2.25.54.PM.movMacOS: DesktopScreen.Recording.2025-01-24.at.2.26.23.PM.mov |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM and tests well
thank you @rayane-djouah :) |
Thanks for following up and for the explanation! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM and tests well
…input-styling [CP Staging] Fix wrong align of TextInput when displaying currency/icon (cherry picked from commit b51369d) (CP triggered by mountiny)
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Cherry-picked to staging by https://github.com/mountiny in version: 9.0.89-3 🚀
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes. |
🚀 Deployed to production by https://github.com/yuwenmemon in version: 9.0.89-8 🚀
|
Explanation of Change
Fixes regression of TextInput styling introduced in my PR: #55343
The core problem was that IF our
TextInput
is actually LiveMarkdown component that is a single line - it cannot live inside a container withflex: 1
because it will get stretched and text inside will not be aligned correctly.I changed the styles into a function, and now it's more explicit what gets what styling.
flex: 1
will be applied to every case that is not LiveMarkdown.Fixed Issues
$ #55662
PROPOSAL:
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
rec-web-textinput.mp4
MacOS: Desktop