-
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
New Marker does not get updated properly while switching between same chat and chat list. #4357
Comments
Triggered auto assignment to @jboniface ( |
Fix Demo: Issue.4357.Fix.Demo.mp4 |
Triggered auto assignment to @TomatoToaster ( |
Triggered auto assignment to @jliexpensify ( |
I'm putting the external label on this so we can get an upwork listing created. I think the problem makes sense and the proposal fits (but I'll have the engineer assigned double check it). |
One thing I will say is that I don't think this method should also be in Does that suggestion make sense? |
I will move that to |
@Santhosh-Sellavel and @TomatoToaster - I'm just catching up as I've been OOO. Given that #4472 has been created, can I close this issue? I assume they're the same issue - correct? |
@jliexpensify |
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! |
Triggered auto assignment to @timszot ( |
Hi @timszot - do you mind checking out Santhosh's proposal here, so we can get him hired? Thanks! |
@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! |
Triggered auto assignment to @robertjchen ( |
Looks like @timszot is OOO, so re-assigning another engineer. |
Ah, I'm actually not familiar enough with this to provide good answers here 🤔 @marcaaron Could I trouble you to see if you have any insight here into @Santhosh-Sellavel 's questions here? #4357 (comment) Thanks! |
@robertjchen you tagged someone else I think instead of me. |
Ah, my mistake- I've updated my tag! Hopefully we'll get some clarifications soon, appreciate your patience. |
I think in this case we should have re-added the |
@marcaaron and @robertjchen - apologies, that was my mistake, I added the Engineering tag. I can post this issue in the channel to see if someone might be willing to take over from Tim? |
Triggered auto assignment to @deetergp ( |
@deetergp Can you answer these questions #4357 (comment) Thanks! |
Does that take into account @parasharrajat's comment here? I had two very minor comments but overall your draft PR pretty good so far. |
Yes partially that it should happen, I'm not clear about why because the view is hidden? |
@parasharrajat Can you elaborate on your earlier comment for @Santhosh-Sellavel? |
Bump for @parasharrajat :) Cheers! |
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. |
Awesome, thanks - should be good to pay Santhosh in 4 more days @deetergp (I think). |
Paid @Santhosh-Sellavel with bonus, apologies for delay, our new automated update messages should prevent this in the future. |
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
Issue 2
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.
New_Marker_Not_Shown_If_Wait_From_Same_Chat.mov
Issue 2
The new marker will not hide even closing and opening multiple times.
New_Marker_Will_Not_Hide_If_Switching_between_Same_Chat_And_List.mov
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?
Proposal
Move the following lines to a new method let's call it
maintainNewMarkerPlace
(suggest new name if needed)App/src/pages/home/report/ReportActionsView.js
Lines 124 to 131 in 87cbc24
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:
View all open jobs on Upwork
The text was updated successfully, but these errors were encountered: