search

Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.33k stars 2.76k forks source link

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

Closed shawnborton closed 1 week ago

shawnborton commented 1 month ago

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 commented 1 month ago

This is related to https://github.com/Expensify/App/issues/41020, since the nav refactor would have to handle both the RHP and main pane navs

trjExpensify commented 1 month ago

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 commented 1 month ago

I could totally get down with that as a v1.

dannymcclain commented 1 month ago

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

luacmartins commented 1 month ago

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

JmillsExpensify commented 1 month ago

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 commented 1 month ago

@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 commented 1 month ago

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

trjExpensify commented 1 month ago

Nice, let's do it. ❤️

JmillsExpensify commented 1 month ago

Awesome, agreed

adamgrzybowski commented 3 weeks ago

Should somebody from SWM or I take care of that?

luacmartins commented 3 weeks ago

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

adamgrzybowski commented 3 weeks ago

@luacmartins ready for review ⬆️

s77rt commented 3 weeks ago

Can you handle the case when editing report fields as well https://github.com/Expensify/App/issues/47554

ikevin127 commented 3 weeks ago

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 commented 3 weeks ago

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

ikevin127 commented 3 weeks ago

I will retest today to make sure the issue was fixed before we merge.

ikevin127 commented 3 weeks ago

Can you handle the case when editing report fields as well https://github.com/Expensify/App/issues/47554

✅ Issue was addressed, tests well https://github.com/Expensify/App/pull/47721#issuecomment-2305463989 and PR will be merged soon 🤞

trjExpensify commented 3 weeks ago

Exciting!

luacmartins commented 3 weeks ago

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 commented 2 weeks ago

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

luacmartins commented 1 week ago

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

melvin-bot[bot] commented 1 week ago

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

trjExpensify commented 1 week ago

Payment is due here!

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 commented 1 week ago

https://github.com/Expensify/App/issues/48716 - here's the follow up issue I have in planning if you're keen for this one, Kevin.

ikevin127 commented 1 week ago

@trjExpensify Offer accepted, thank you!

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

trjExpensify commented 1 week ago

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

ikevin127 commented 1 week ago

@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 commented 1 week ago

Yep, I just paid it.

trjExpensify commented 1 week ago

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

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