App: New Marker does not get updated properly while switching between same chat and chat list.
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
There are two issues but are similar and the same fix would solve both. So grouped as one.
Action Performed:
Issue 1
- Open any user chat.
- Tap the back arrow to navigate to the chat list.
- Now wait for a new message from the user.
- After receiving the message open the chat, the new marker is not shown.
Issue 2
- Open any other chat.
- Tap the back arrow to navigate to the chat list.
- Now wait for a new message from the user.
- After receiving the message open the chat, the new marker will be shown now.
- But go to the chat list, open chat again. The new marker will not hide.
Expected Result:
Issue 1
The new marker should be shown.
Issue 2
The new marker should be hidden while open the chat for the second time.
Actual Result:
Issue 1
The new marker is not shown.
Issue 2
The new marker will not hide even closing and opening multiple times.
Workaround:
Can the user still use Expensify without this being fixed? Yes
Issue 1
Need wait from another user, then a new marker will be shown.
Issue 2
New marker only if we switch to other chat and navigate back here.
Platform:
Where is this issue occurring?
- iOS
- Android
- Mobile Web
Proposal
Move the following lines to a new method let’s call it maintainNewMarkerPlace (suggest new name if needed)
https://github.com/Expensify/App/blob/87cbc24238115426df8c16111d790f77930b9bfb/src/pages/home/report/ReportActionsView.js#L124-L131
then bind the method.
And invoke in the componentDidMount method (since we moved those lines from which is intended to happen).
And in shouldComponentUpdate method as below,
Version Number: Logs: https://stackoverflow.com/c/expensify/questions/4856 Notes/Photos/Videos: Any additional supporting documentation Expensify/Expensify Issue URL:
About this issue
- Original URL
- State: closed
- Created 3 years ago
- Comments: 43 (22 by maintainers)
@jliexpensify I feel something is fishy going. The changes I proposed do not work for me now. Considering #4472 depend’s on this issue solution and related. I would revisit the changes and update a new proposal soon, max within 3 - 4 days. If not I’ll let you lets make it open for others also thanks!
I think PR is already merged and I am sorry, I have no detailed explanation for that . Rest code must be saying a better story than mine.
@deetergp Can you answer these questions https://github.com/Expensify/App/issues/4357#issuecomment-907420704 And Review my draft PR #4887
Thanks!
@jliexpensify I see the assigned engineer is OOO until sep 7 can you assign someone else, thanks!
@timszot or @jliexpensify assign this to me!
Got offer @jliexpensify
Hired @Santhosh-Sellavel , then Upworks crashed:
Let me know if you didn’t receive an offer, and I’ll look into it further!
@Santhosh-Sellavel A PR shouldn’t be opened until you’ve proposed a solution and have been hiring in Upwork per our contributing guidelines. On that note… @jliexpensify let’s hire @Santhosh-Sellavel for this one!
@timszot Sorry for the delayed notice. Working on it along with a couple of new items discussed here Will update the status by Monday again on progress thanks!
Thanks for the update @Santhosh-Sellavel - have added a [Hold] to the title and updated the GH to weekly to give you a little more time. Feel free to tag Tim when you’ve got a solution, or let us know if this GH can be closed!
Sorry for the delay, was checking a few things with @TomatoToaster - posted to Upworks: https://www.upwork.com/ab/applicants/1424866489594466304/job-details
@Santhosh-Sellavel - thanks for your patience. Please feel free to apply in Upworks!
@jliexpensify IMO, they are not same issue.
I will move that to
componentDidUpdateit makes more sense for me too. @TomatoToasterOne thing I will say is that I don’t think this method should also be in
shouldComponentUpdate. That function should only handle the boolean logic on whether the props changing is relevant to reruncomponentDidUpdate. Instead you should move that call ofthis.maintainNewMarkerPlace()tocomponentDidUpdate. I think it makes more sense for it to be there since you’re already telling it to runcomponentDidUpdatewith thereturn truein the second snippet you have.Does that suggestion make sense?