-
Notifications
You must be signed in to change notification settings - Fork 3k
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
(2/2) Add support for all reportAction types in ChatListItem - use PureReportActionItem in ChatListItem #54228
base: main
Are you sure you want to change the base?
(2/2) Add support for all reportAction types in ChatListItem - use PureReportActionItem in ChatListItem #54228
Conversation
…x/51296-chat-list-item
…x/51296-chat-list-item
@luacmartins @allgandalf this is draft PR for the second part. Will the logic to show icons the same between reportactionitem and chatlistitem? PureReportActionItem might render ReportActionItemSingle and use getReportActionActorAccountID, to determine the icons from personalDetails, but SearchReportAction doesn't have ownerAccountID, actorAccountID, etc to fullfill the logic. will |
@wildan-m Search already returns |
PR to add |
…x/51296-chat-list-item
@luacmartins @allgandalf There is a new ESLint rule that enforces the use of a default value of '0' or undefined instead of '-1'. I am concerned that this change could potentially cause regressions, especially because there are numerous occurrences in the PureReportActionItem, ReportActionItemSingle and their subcomponents. Should this be addressed in this pull request?
|
Let's open a separate PR for that and get it merged first, so if there are regressions it's isolated to that one PR and it's easy to revert |
@wildan-m can you please fix the workflows |
…x/51296-chat-list-item
…x/51296-chat-list-item
…x/51296-chat-list-item
…x/51296-chat-list-item
Explanation of Change
Fixed Issues
$
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
MacOS: Desktop