Skip to content
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

[HOLD for payment 2024-09-06][Search v2.1] Keep user on Search page if expense is created from Search page #47179

Closed
shawnborton opened this issue Aug 9, 2024 · 30 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2

Comments

@shawnborton
Copy link
Contributor

shawnborton commented Aug 9, 2024

Problem

If you are on the Search page and you create an expense, you get taken back to the Inbox page. This is jarring because the user would actually expect to remain on the Search page and see the expense show up in the Search table.

Solution

Let's make it so that if you were on the Search page and you create an expense, you land back on the Search page. We could make it so that you just return to the Search table and see the table populate with the new expense, or we could open up the report or workspace chat in the RHP.

Note: right now we have the ability to open up a report or expense in the RHP on Search but I don't think we can open up a workspace chat in the RHP on the Search page?

cc @Expensify/design @trjExpensify @JmillsExpensify @luacmartins

@trjExpensify - can you file this one where you want it?

Issue OwnerCurrent Issue Owner: @trjExpensify
@luacmartins
Copy link
Contributor

This is related to #41020, since the nav refactor would have to handle both the RHP and main pane navs

@trjExpensify
Copy link
Contributor

Should we put this in #wave-control then with everything else Search related?

Let's make it so that if you were on the Search page and you create an expense, you land back on the Search page. We could make it so that you just return to the Search table and see the table populate with the new expense, or we could open up the report or workspace chat in the RHP.

Maybe the v1 pre-refactor is landing back on the table with the new expense row created highlighted for emphasis or something?

@shawnborton
Copy link
Contributor Author

I could totally get down with that as a v1.

@dannymcclain
Copy link
Contributor

Same, I think that'd be nice as a v1.

@luacmartins
Copy link
Contributor

@adamgrzybowski @WojtekBoman since you're more familiar with navigation logic, how involved do you think it'd be to implement this?

@JmillsExpensify
Copy link

JmillsExpensify commented Aug 14, 2024

Zooming out a bit, given the navigation refactors required for this issue, I think I'd still choose prioritizing Search 2.3 and 2.4 ahead of this initiative. Still down to see how "big" of an effort it is regardless though.

@adamgrzybowski
Copy link
Contributor

@luacmartins I did some quick research and we call the dismissModal function with the reportID argument on confirm.

If we call this without argument, the user should stay on the search page.

We can conditionally remove the argument if the topmost central pane is the search page.

@luacmartins
Copy link
Contributor

That sounds good. If it's an easy fix, let's go for it.

@luacmartins luacmartins added Weekly KSv2 and removed Monthly KSv2 labels Aug 14, 2024
@luacmartins luacmartins changed the title Keep user on Search page if expense is created from Search page [Search v2.1] Keep user on Search page if expense is created from Search page Aug 14, 2024
@trjExpensify
Copy link
Contributor

Nice, let's do it. ❤️

@JmillsExpensify
Copy link

Awesome, agreed

@adamgrzybowski
Copy link
Contributor

Should somebody from SWM or I take care of that?

@luacmartins
Copy link
Contributor

Yea, would love if you could work on this one @adamgrzybowski. You're already assigned to the issue, so please go ahead.

@adamgrzybowski
Copy link
Contributor

@luacmartins ready for review ⬆️

@s77rt
Copy link
Contributor

s77rt commented Aug 22, 2024

Can you handle the case when editing report fields as well #47554

@ikevin127
Copy link
Contributor

ikevin127 commented Aug 22, 2024

I dropped a comment in the PR to ask whether we're going to address that issue in the already opened PR before merge.

@adamgrzybowski
Copy link
Contributor

Hey, I added the changes for the EditReportFiledPage.tsx. Can someone please double check that it is working now? Thanks! 🙇

@ikevin127

This comment was marked as outdated.

@ikevin127
Copy link
Contributor

Can you handle the case when editing report fields as well #47554

✅ Issue was addressed, tests well #47721 (comment) and PR will be merged soon 🤞

@trjExpensify
Copy link
Contributor

Exciting!

@luacmartins
Copy link
Contributor

Coming from this comment, we should add the new transaction to the list. We don't optimistically create this data in the search Onyx key and we should keep it that way. 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.

@trjExpensify trjExpensify self-assigned this Aug 27, 2024
@trjExpensify
Copy link
Contributor

PR hit staging yesterday, assigning myself for my sins BZ duty.

@luacmartins
Copy link
Contributor

PR was deployed to prod in v9.0.26-6.

@luacmartins luacmartins changed the title [Search v2.1] Keep user on Search page if expense is created from Search page [HOLD for payment 2024-09-06][Search v2.1] Keep user on Search page if expense is created from Search page Sep 2, 2024
@luacmartins luacmartins added Daily KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Reviewing Has a PR in review Weekly KSv2 labels Sep 2, 2024
@melvin-bot melvin-bot bot added the Overdue label Sep 4, 2024
Copy link

melvin-bot bot commented Sep 5, 2024

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

@trjExpensify
Copy link
Contributor

Payment is due here!

  • $250 to @ikevin127 for the C+ review (offer sent!).

As a follow-up, I think we should consider some kind of highlight/flash of the newly added expense row in the table. Maybe @ikevin127 you'd fancy taking that on?

@trjExpensify
Copy link
Contributor

#48716 - here's the follow up issue I have in planning if you're keen for this one, Kevin.

@ikevin127
Copy link
Contributor

@trjExpensify Offer accepted, thank you!

Sure, I don't mind reviewing the other issue as well ✌️

@trjExpensify
Copy link
Contributor

Oh right, I was more so wondering if you want to hatch a plan and implement it. :)

@ikevin127
Copy link
Contributor

@trjExpensify I can do that as well, will look into the issue today and try to come up with a proposal.

In the meantime, do you mind finalizing the payment on this issue ? I accepted the offer, thank you!

@trjExpensify
Copy link
Contributor

Yep, I just paid it.

@trjExpensify
Copy link
Contributor

will look into the issue today and try to come up with a proposal.

Sounds good, let's chat on the other issue. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2
Projects
Status: Done
Development

No branches or pull requests

8 participants