-
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
Open reports at first unread action #51366
base: main
Are you sure you want to change the base?
Conversation
20bb3d2
to
c811ef7
Compare
fb6a2fb
to
3b03cef
Compare
3b03cef
to
b88cba8
Compare
b88cba8
to
13de5ad
Compare
Oh I guess I am not sure how to assign someone from the @Expensify/design team to the PR, but I think they can help with your question #51366 (comment) |
I personally like this, but I think I would prefer to let an older message slightly peak out from above so there is a little more context that you are starting from a certain point. So basically what you have but maybe push down the starting point by like 56px or 64px or something like that. |
Yeah I like that too Shawn! |
+1 to that. Always like it when we peek content to show that there's more there |
@janicduplessis First of all, props for picking up this veteran difficulty level issue, doing such great work on it so far and getting it so close to completion 🚀 👋 Please tag me before merging this in case I don't get notified because I'd like to run some tests myself to make sure this works as expected, I have 2 scenarios in mind:
Note: I'm one of the contributors which attempted to fix this from FE only but ultimately figured out it won't work and suggested a BE + FE implementation discussion months ago before Rory went on parental leave. How did I get here ? Another C+ reviewer asked just yesterday about an issue on Slack, which will basically be fixed by this PR - that's how I remembered this 1 year old issue and decided to join the testing effort as well since I have some context on this. Note: No compensation needed, just want to help 👍 |
Details
Implements opening reports at the last unread message. This builds on top of #52149 to be able to start our list at any item. This removes a few assumptions that open report will always open at the newest action.
Fixed Issues
$ #35011
PROPOSAL:
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.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
Screen.Recording.2025-01-27.at.23.42.32.mov
Android: mWeb Chrome
Screen.Recording.2025-01-27.at.23.37.31.mov
iOS: Native
Screen.Recording.2025-01-27.at.22.56.37.mov
iOS: mWeb Safari
Screen.Recording.2025-01-27.at.23.02.45.mov
MacOS: Chrome / Safari
Screen.Recording.2025-01-27.at.22.33.19.mov
MacOS: Desktop
Screen.Recording.2025-01-27.at.23.32.32.mov