Expense-Scan and Manual Options Partially Hidden at Maximum Font Size#75493
Conversation
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
…6/text-are-partially-hidden
…6/text-are-partially-hidden
|
@QichenZhu Please 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] |
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Good from a product perspective 👍
|
@thelullabyy, the lint error is not related, but there are some warnings we should fix. |
Good catch! I've noticed that for a while. If it's not worth a separate issue, we can fix it along the way. Let me know what works best. |
|
If you know the fix and can do it now, I'd be very grateful 😇 |
…6/text-are-partially-hidden
Reviewer Checklist
Screenshots/Videos |
@thelullabyy thanks for working on it! @dubielzyk-expensify do the screenshots below look good to you?
|
|
Looks good to me 👍 |
| styles.textMicroSupportingPadding, | ||
| styles.pre, | ||
| styles.flexShrink1, | ||
| {color: theme.danger}, |
There was a problem hiding this comment.
NAB do we have a util for inline colors?
There was a problem hiding this comment.
…6/text-are-partially-hidden
| import ConfirmModal from '@components/ConfirmModal'; | ||
| import {DelegateNoAccessContext} from '@components/DelegateNoAccessModalProvider'; | ||
| import Icon from '@components/Icon'; | ||
| import * as Expensicons from '@components/Icon/Expensicons'; |
There was a problem hiding this comment.
@thelullabyy would you mind fixing this import lint warning in your PR?
There was a problem hiding this comment.
Sure, I will fix it today
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/grgia in version: 9.2.71-0 🚀
|
|
@grgia @thelullabyy, this was reverted because of the spacing issue, but that is actually expected. Should we revert the revert? |
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.2.71-5 🚀
|
|
@QichenZhu @grgia I will revert the revert shortly. Please let me know if we have any other issues ^ |
|
The revert PR is ready here |































Explanation of Change
Fixed Issues
$ #74676
PROPOSAL: #74676 (comment)
Tests
Pre-condition: Set app to XXX maximum font size
(All UI elements, error messages, and tags should be fully visible at maximum font size)
Offline tests
QA Steps
Pre-condition: Set app to XXX maximum font size
(All UI elements, error messages, and tags should be fully visible at maximum font size)
PR 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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop