[HOLD for payment 2024-10-11] [HOLD for payment 2024-10-10] [$250] [Search v2.2] Reload the page and highlight the newly added expense when adding a new expense from the search page

[trjExpensify]

Follow-up to here.

Two items to add:

1. When the new expense is added to the search page, trigger a reload of the page so it can appear in the table.

I think we might need to just trigger a reload of the page given the existing filters may or may not show the new transation.

2. Highlight the newly added expense row in the table when it appears

For this, we were thinking it could borrow from the workspace settings when you enable a new feature in More features, and the row gets highlighted in the LHN briefly.

Once we've done that, we'll add this regression test for Applause:

1. Switch to the search page tab
2. Click the global create button > submit expense > submit an expense
3. Verify you remain on the search page
4. Verify the search page reloads and the expense appears in the table
5. Verify the newly added expense row is briefly highlighted to draw attention to it

CC: @Expensify/design @luacmartins @adamgrzybowski @ikevin127

Issue Owner

Current Issue Owner: @

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021838256578746413929
• Upwork Job ID: 1838256578746413929
• Last Price Increase: 2024-09-23
• Automatic offers:
• ikevin127 | Reviewer | 104270100
• ishpaul777 | Contributor | 104270102

Issue Owner

Current Issue Owner: @trjExpensify

[melvin-bot[bot]]

Current assignee @trjExpensify is eligible for the Bug assigner, not assigning anyone new.

[melvin-bot[bot]]

Triggered auto assignment to @dubielzyk-expensify (Design), see these Stack Overflow questions for more details.

[trjExpensify]

Can we get a mock of the row highlighted to include here, please? Thanks!

[ikevin127]

♻️ Will look into this tomorrow, the plan being to come up with a proposal and some videos for the design team to review 👍

[dubielzyk-expensify]

Animation 1: Just color fade

https://github.com/user-attachments/assets/3c3ce860-7b9a-4cf8-a95f-2e6b5d0a6c49

Animation 2: Slide and color fade

https://github.com/user-attachments/assets/72088950-1c40-4f34-9c52-00226c7b4596

Animation 3: Color fade and stay

https://github.com/user-attachments/assets/261bd259-daca-4ede-b4ba-97affc886652

No animation: Use highlight state color

We do use a highlight color on chats but I'm personally not a huge fan of this due to how muddy it looks. If we apply it here I don't think it looks great:

cc @Expensify/design team for thoughts

[shawnborton]

Slide and color fade is by far my favorite! If that's not possible, I think I lean towards just the color fade. Fade and stay feels a bit weird in that it's almost like the expense row is selected. And I agree about the muddy color 🙈

[dannymcclain]

Slide and color fade is by far my favorite! If that's not possible, I think I lean towards just the color fade.

100% agree. Slide and color fade is :chefkiss: but if that's not feasible, color fade is still great.

@dubielzyk-expensify what color are you using for the "highlight"? I almost wonder if we should make it a bit more obvious since we just want a quick little fade?

PS. It's a little sad to me that we have a highlight color variable but we don't want to use it because it doesn't look great... I feel like it might be worth revisiting and fixing that color because this situation is essentially exactly what that variable is for: a highlight.

[shawnborton]

Yeah, that's a fair sentiment. I think the reality is that it looks bad on dark mode, but it looks pretty normal for dark mode themes. In light mode, I think it feels better and uses an actual brand color right?

[dannymcclain]

I think the reality is that it looks bad on dark mode, but it looks pretty normal for dark mode themes.

Yeah I do agree with that.

In light mode, I think it feels better and uses an actual brand color right?

Yes, in light mode it's just set to yellow-100 (and it looks great haha).

[dubielzyk-expensify]

Yeah agree that in light mode it looks great. Maybe we just revisit the dark mode highlight cause I think you're right Danny that we should use this semantic color since it has a purpose.

Slide looks good but I made a version without it cause I worried about performance if the list is huge. I'm happy with either. I think the stay is my least preferred. We can do the normal fade and just delay the fade if we want anyways. Also with the stay version we'd have to address the checkbox and receipt color stuff which I really don't wanna do haha.

@dubielzyk-expensify what color are you using for the "highlight"? I almost wonder if we should make it a bit more obvious since we just want a quick little fade?

I used row-highlight in that example cause I didn't want it to cross over into select land (button), but I agree that it's a tad subtle.

Here's updated slide-in with highlight color for dark and light mode:

https://github.com/user-attachments/assets/32020c33-c1d0-4829-bc39-dc267badc148

https://github.com/user-attachments/assets/ce234859-9d38-4f2d-95cd-427e09ef5781

[ikevin127]

Thanks for confirming the animation we want here! 🚀

♻️ Made some progress today functionality wise (see video below), the only thing left to do is to integrate the animation which so far it looks like everybody is in favor of the Animation 2: Slide and color fade, please let me know if we want something different - anyhow I will come back tomorrow with the final version including animation present a new video for confirmation 👍

Video

https://github.com/user-attachments/assets/5857e4ae-cec7-430b-a469-312233dc0953

[dubielzyk-expensify]

Awesome. Yeah let's wait until @dannymcclain and @shawnborton have given their thoughts 😄

[shawnborton]

I definitely appreciate how the slide fade with highlight color makes things more obvious and is a bit more connected to the color we use for comment highlighting. Maybe we go with that and then follow up and pick a better color for dark mode? I can also get down with just the slide fade using border color too.

[dubielzyk-expensify]

100% down for that. Let me add a follow-up task for myself to explore the dark mode highlight color. I think that's the right call, though keen to hear if @dannymcclain agrees?

[trjExpensify]

Assigning @ikevin127, forgot to do that. Also assigning @luacmartins for the internal review portion of the PR when ready.

[dannymcclain]

I totally agree! That sounds like a great plan to me. And I also think that if the slide animation can't work because of performance, just doing the highlight animation will still be great.

[ikevin127]

♻️ Update: I've been going at it for the past 2 days trying to find a way to implement the animation and still haven't figured it out - I'll continue today and if by EOD I don't have it figured, we'll have to pull somebody in who worked on animation before and have them take over.

[luacmartins]

@ikevin127 have you checked out the logic in HighlightableMenuItem, we use that when we enable workspace features on the More features page

https://github.com/user-attachments/assets/e6163731-24f7-4d86-a53e-c284ae0f5b2f

[ikevin127]

@luacmartins Yes, that's where I took inspiration from and I managed to figure out how to trigger the animation - now I'm working on the styling, I'm close 👍

[ikevin127]

I got it working but there seems to be a frame-drop while the height animates and I think it happens because I set the animation height to fixed 64px even though the row itself doesn't have a fixed height but instead it gets to 64px from the content and padding of the row.

This is how it looks

https://github.com/user-attachments/assets/4a273790-d388-458b-99a8-b7d827e542b5

Will open the PR but I'm not liking that frame-drop so we might remove the height animation and only keep the highlight which would look something like this:

https://github.com/user-attachments/assets/ab22eba5-6e7b-437d-909a-4869fb5b9179

cc @Expensify/design

[dubielzyk-expensify]

I like it. Can you change the highlight color to be the same color we use for highlighting chat?

Also can you give me the numbers you're using for each delay etc in ms? Think we might wanna increase the delay before the highlight color starts to fade, but I'm not 100% sure yet :)

[shawnborton]

Nice! Yeah, I agree with Jon - let's see it in our highlight color as well but it seems like you've got the general UX part figured out which is great.

[trjExpensify]

Agreed, great progress here!

[ikevin127]

Also can you give me the numbers you're using for each delay etc in ms?

@dubielzyk-expensify Sure, I'm using the already existing useAnimatedHighlightStyle hook's default values:

https://github.com/Expensify/App/blob/4039a12cfb06f2bf47d11f86a6da6482e61fc05a/src/hooks/useAnimatedHighlightStyle/index.ts#L40-L50

which are (in ms):

    itemEnterDelay = CONST.ANIMATED_HIGHLIGHT_ENTRY_DELAY, // 50
    itemEnterDuration = CONST.ANIMATED_HIGHLIGHT_ENTRY_DURATION, // 300
    highlightStartDelay = CONST.ANIMATED_HIGHLIGHT_START_DELAY, // 10
    highlightStartDuration = CONST.ANIMATED_HIGHLIGHT_START_DURATION, // 300
    highlightEndDelay = CONST.ANIMATED_HIGHLIGHT_END_DELAY, // 800
    highlightEndDuration = CONST.ANIMATED_HIGHLIGHT_END_DURATION, // 2000

and my changes / additions to the hook are highlight background / foreground color:

    const animatedHighlightStyle = useAnimatedHighlightStyle({
        borderRadius: variables.componentBorderRadius,
        shouldHighlight: item?.highlighted ?? false,
        highlightColor: theme.messageHighlightBG,
        backgroundColor: theme.highlightBG,
    });

This is how it looks with the yellow chat highlight for both dark / light themes, without the glitchy height (64px) animation:

https://github.com/user-attachments/assets/9e0db8b0-74be-4c13-9d10-3a4f70b023f1

[luacmartins]

@ikevin127 do you have a draft PR with the changes above? If so, could you link this issue to it?

[ikevin127]

@luacmartins Just opened the PR https://github.com/Expensify/App/pull/49192, will tag design team and we can move the disucssion there for polishing 👍

[dubielzyk-expensify]

Thanks @ikevin127 . Tested it in the PR and it looks great 👏 Added a feedback on not getting it from the empty state reliably

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~021838256578746413929

[melvin-bot[bot]]

Current assignee @ikevin127 is eligible for the External assigner, not assigning anyone new.

[trjExpensify]

Just adding the External label to get a job created for Kevin here.

[melvin-bot[bot]]

@trjExpensify, @luacmartins, @ikevin127, @dubielzyk-expensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[ikevin127]

♻️ The PR is about to be merged, just resolved latest conflicts and C+ review passed.

Edit: PR was merged.

[melvin-bot[bot]]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[melvin-bot[bot]]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot[bot]]

The solution for this issue has been :rocket: deployed to production :rocket: in version 9.0.43-6 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:

• https://github.com/Expensify/App/pull/49192

If no regressions arise, payment will be issued on 2024-10-10. :confetti_ball:

For reference, here are some details about the assignees on this issue:

• @ikevin127 requires payment (Needs manual offer from BZ)

[melvin-bot[bot]]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [ ] [@ikevin127] The PR that introduced the bug has been identified. Link to the PR:
• [ ] [@ikevin127] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [ ] [@ikevin127] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [ ] [@ikevin127] Determine if we should create a regression test for this bug.
• [ ] [@ikevin127] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [ ] [@trjExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[ishpaul777]

👋 Please assign me here i reviewed https://github.com/Expensify/App/pull/49192

[melvin-bot[bot]]

📣 @ikevin127 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job

[melvin-bot[bot]]

📣 @ishpaul777 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

[melvin-bot[bot]]

The solution for this issue has been :rocket: deployed to production :rocket: in version 9.0.44-12 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:

• https://github.com/Expensify/App/pull/50028

If no regressions arise, payment will be issued on 2024-10-11. :confetti_ball:

For reference, here are some details about the assignees on this issue:

• @ikevin127 requires payment automatic offer (Reviewer)
• @ishpaul777 requires payment automatic offer (Contributor)

[melvin-bot[bot]]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [ ] [@ikevin127 / @ishpaul777] The PR that introduced the bug has been identified. Link to the PR:
• [ ] [@ikevin127 / @ishpaul777] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [ ] [@ikevin127 / @ishpaul777] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [ ] [@ikevin127 / @ishpaul777] Determine if we should create a regression test for this bug.
• [ ] [@ikevin127 / @ishpaul777] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [ ] [@trjExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[ikevin127]

• [x] [@ikevin127] The PR that introduced the bug has been identified. Link to the PR: https://github.com/Expensify/App/pull/47721
• [x] [@ikevin127] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: No need as this issue is a follow-up of issue: https://github.com/Expensify/App/issues/47179.
• [x] [@ikevin127] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A.
• [x] [@ikevin127] Determine if we should create a regression test for this bug.
• [x] [@ikevin127] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Proposal

1. Switch to the Search page tab.
2. Click the global create button (FAB) > Submit expense > Submit an expense.
3. Verify you remain on the Search page.
4. Verify that the newly added expense appears in the table and the row is briefly highlighted to draw attention to it.

Do we agree 👍 or 👎.

[trjExpensify]

Regression test created. Payment summary as follows:

• $250 to @ikevin127 for the fix
• $250 to @ishpaul777 for the C+ review

Paid, closing!