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

[$250] [Search v2.2] Hide mobile Search nav button + status tabs on scrollDown, but reveal on scrollUp #48019

Open luacmartins opened 2 weeks ago

luacmartins commented 2 weeks ago

Problem On mobile, the Search page occupies a lot of fixed space between the top workspace switcher, the page navigation button, the status tabs, and the bottom fixed tabs. This doesn't lave a whole lot of vertical space remaining to view the expenses page.

Solution: Let's make it so that the nav button and status tabs scroll the page with the content, but are revealed as soon as the user scrolls back up. Here is a rough prototype of the action:

https://github.com/user-attachments/assets/1e7f7599-6990-454c-9846-f95c98c42a2f

Note that in the prototype, we're scrolling the button + tabs back into view at the same speed as the content, but I could see where it might even make sense to just animate that area back in as soon as we detect "one scroll" going back up.

cc @Expensify/design @luacmartins for visibility. I think this is good #wave-control polish.

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01ee6fbdd84bda73a1
  • Upwork Job ID: 1828223707737004666
  • Last Price Increase: 2024-08-27
  • Automatic offers:
    • rayane-djouah | Contributor | 103694364
Issue OwnerCurrent Issue Owner: @rayane-djouah
melvin-bot[bot] commented 2 weeks ago

Triggered auto assignment to @kadiealexander (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details. Please add this Feature request to a GH project, as outlined in the SO.

melvin-bot[bot] commented 2 weeks ago

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

melvin-bot[bot] commented 2 weeks ago

Triggered auto assignment to Contributor-plus team member for initial proposal review - @situchan (External)

melvin-bot[bot] commented 2 weeks ago

πŸ“£ @rayane-djouah πŸŽ‰ 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 πŸ“–

nkdengineer commented 2 weeks ago

@luacmartins Can you share the video in OP again, I can't see it in the OP.

sachinsiddhu112 commented 2 weeks ago

I am unable to see provided video or screenshot.Please provide a video or screenshot of issue.

melvin-bot[bot] commented 2 weeks ago

πŸ“£ @sachinsiddhu112! πŸ“£ Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Screen Shot 2022-11-16 at 4 42 54 PM Format:
    Contributor details
    Your Expensify account email: <REPLACE EMAIL HERE>
    Upwork Profile Link: <REPLACE LINK HERE>
SzymczakJ commented 2 weeks ago

Hey! I’m Jakub Szymczak from Software Mansion, an expert agency, and I’d like to work on this issue!

SzymczakJ commented 2 weeks ago

When working on this issue I got one thought concerning StatusBar during selection mode. I think we shouldn't show it during selection mode:

  1. When clicking on anything on status bar Selection mode is cancelled

https://github.com/user-attachments/assets/e0d1c53a-1a56-440c-99a4-57774180e709

  1. We have already hidden filters button and Type selector, so it doesn't make sense to give user option to choose Status
  2. It's cluttering the screen
  3. Removing this from selection mode would make this task easier to implement.

I could remove StatusBar from selection mode and put this change in PR that would be linked to this task WDYT @luacmartins

shawnborton commented 2 weeks ago

Yeah, I think I agree with that. cc @Expensify/design for thoughts there too, but what you laid out above makes sense to me.

dannymcclain commented 2 weeks ago

Totally agreeβ€”we never intended for the status options to be present in Select mode. Here's a quick grab from the Figma file: CleanShot 2024-08-28 at 08 16 07@2x

SzymczakJ commented 2 weeks ago

How do you guys like it? @dannymcclain @shawnborton @luacmartins

https://github.com/user-attachments/assets/09e7ae47-6b65-4f6c-abca-40eae1addf26

shawnborton commented 2 weeks ago

Looking pretty good!

I think we should remove the gap below the status tabs though so it's flush with the content below it. Let's add that gap to the top of the scrolling part above the rows: CleanShot 2024-08-28 at 09 45 48@2x

Looks like the status tabs are cut off too btw: CleanShot 2024-08-28 at 09 46 40@2x

As for the scroll action itself, I was thinking that the top nav button + status tabs would scroll away at the same pace as the row content. Right now it seems like we are just hiding that area as soon as we detect a single scroll, is that correct?

dannymcclain commented 2 weeks ago

Once again, totally agree with all of Shawn's comments.

I think it should scroll away at the same pace as the content when you're at the top, but when you're scrolled down and scroll back up, I do think it should come in with a snappier animation than the regular scroll since we're showing it "in the middle" of a list of item. And then if you scroll down again (without going all the way to the top) I think it would be okay for it to go away with a bit more juice than the normal scroll (since again, it's not really in its home/resting place). Does that make any sense at all?? πŸ˜… πŸ˜΅β€πŸ’«

shawnborton commented 2 weeks ago

Haha makes total sense to me, I agree!

SzymczakJ commented 2 weeks ago

Right now it seems like we are just hiding that area as soon as we detect a single scroll, is that correct?

Yeah, that's how it's currently implemented, as it's the easier solution and I wanted to know your opinion. I'll try to adjust to your comments, but doing so is pretty hard, because of the limitations of SelectionList, which we are forced to use(the outcome may be laggy or not so smooth). Anyway I'll ping you when I have some new!

SzymczakJ commented 2 weeks ago

https://github.com/user-attachments/assets/280f89c7-dad9-4d4a-b788-25b1c63d2cb2

This is what I managed to come up with:

  1. When user scrolls down the tab is hiding with the same speed as the user scrolls.
  2. When user scrolls up the drawer is put up to full height at a constant speed WDYT @dannymcclain @shawnborton
shawnborton commented 2 weeks ago

Feels solid to me! I would get rid of the gap below the status tabs though, and have that be part of the gap on top of the rows, if that makes sense.

rayane-djouah commented 2 weeks ago

Not overdue

SzymczakJ commented 2 weeks ago

I'm not sure what do you mean πŸ˜…. Do you mean to remove padding from Status component and put it to the top of the list? If yes then this looks like this, when you scroll a little bit down(we no longer see the padding because we scrolled pass it).

Screenshot 2024-08-29 at 15 14 55
shawnborton commented 2 weeks ago

Yup! Perhaps we could keep the padding but reduce it to 8px or something - mostly just thinking that we had too much padding below it previously and it felt a bit clunky?

dannymcclain commented 2 weeks ago

mostly just thinking that we had too much padding below it previously and it felt a bit clunky?

Yeah I agree. I like your suggestion of trying 8px to see what that's like. We just need a little bit of separation.

SzymczakJ commented 2 weeks ago
Screenshot 2024-08-30 at 10 10 24

Reduced padding to 8 and I think it looks neat πŸ˜ƒ

shawnborton commented 2 weeks ago

Can you share a video of that?

SzymczakJ commented 2 weeks ago

https://github.com/user-attachments/assets/c654c9b4-54b6-4b65-b6d3-874ad47e7db5

Here you go

shawnborton commented 2 weeks ago

Cool, I think we just need to add some padding back to the top of the rows so that we get a larger gap between the status tabs and the rows: CleanShot 2024-08-30 at 11 48 24@2x

Let me know if that makes sense. We probably need to add like 12px there to make up for the 12px we shaved off from the tabs area.

SzymczakJ commented 2 weeks ago

Are talking about something like this?

https://github.com/user-attachments/assets/e1b7cd59-5e12-45b3-b805-bfdf9875fdc3

shawnborton commented 2 weeks ago

Yup, that spacing looks correct.

SzymczakJ commented 1 week ago

There was a need for small refactor, but this should be ready for C+ review tomorrow!

melvin-bot[bot] commented 1 week ago

@luacmartins, @kadiealexander, @rayane-djouah, @SzymczakJ Huh... This is 4 days overdue. Who can take care of this?