-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Update inbox tooltip and add expense report filter type tooltip #58721
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
Update inbox tooltip and add expense report filter type tooltip #58721
Conversation
@ishpaul777 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] |
…igration-or-isUserPolicyEmployee
ping me when this ready : ) |
@ishpaul777 Do you know the correct Onyx command to migrate an account? |
This worked. Thanks! |
Reviewer Checklist
Screenshots/Videos |
src/languages/es.ts
Outdated
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.
i assume Translation confirmation needed
name: EXPENSE_REPORTS_FILTER, | ||
priority: 2000, | ||
shouldShow: ({shouldUseNarrowLayout, isUserPolicyAdmin, hasBeenAddedToNudgeMigration}: ShouldShowConditionProps) => | ||
!shouldUseNarrowLayout && isUserPolicyAdmin && hasBeenAddedToNudgeMigration, |
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.
lets confirm if we would like to show this on mobile or not
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.
I followed the RENAME_SAVED_SEARCH
tooltip, which is displayed in the same location and hidden in smaller layouts:
App/src/components/ProductTrainingContext/TOOLTIPS.ts
Lines 45 to 53 in f48ee32
[RENAME_SAVED_SEARCH]: { | |
content: [ | |
{text: 'productTrainingTooltip.saveSearchTooltip.part1', isBold: true}, | |
{text: 'productTrainingTooltip.saveSearchTooltip.part2', isBold: false}, | |
], | |
onHideTooltip: (isDismissedUsingCloseButton = false) => dismissProductTraining(RENAME_SAVED_SEARCH, isDismissedUsingCloseButton), | |
name: RENAME_SAVED_SEARCH, | |
priority: 1250, | |
shouldShow: ({shouldUseNarrowLayout}) => !shouldUseNarrowLayout, |
…igration-or-isUserPolicyEmployee
Asked for translations confirmation here: https://expensify.slack.com/archives/C01GTK53T8Q/p1744738354719109 |
We have some conflicts |
|
@ishpaul777 This is ready for review |
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.
Nice, let's move forward!!
@rayane-d code is good, but it seems the PR is missing videos for most platforms, can you provide those before we move forward with merge? |
…igration-or-isUserPolicyEmployee
@MarioExpensify Done! |
Thank you @rayane-d, @Expensify/design can you take a look before we move forward? |
Screenshots look good to me! |
@ishpaul777 @rayane-d could you help with the step?
What should we do here? |
@kavimuru Here are the steps for migrating the account:
|
🚀 Deployed to staging by https://github.com/MarioExpensify in version: 9.1.33-0 🚀
|
🚀 Deployed to staging by https://github.com/MarioExpensify in version: 9.1.35-1 🚀
|
🚀 Deployed to staging by https://github.com/MarioExpensify in version: 9.1.36-3 🚀
|
🚀 Deployed to staging by https://github.com/MarioExpensify in version: 9.1.37-1 🚀
|
🚀 Deployed to production by https://github.com/thienlnam in version: 9.1.37-3 🚀
|
Explanation of Change
Fixed Issues
$ #57045
The changes for the issue are divided into multiple PRs, as outlined in this comment: #57045 (comment)
Tests
Test case 1:
Test case 2:
Offline tests
QA Steps
Same as tests
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
MacOS: Desktop