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

[Wave 8] [Ideal Nav] Update Account Settings pages to use the new card styles #32952

Closed
trjExpensify opened this issue Dec 13, 2023 · 34 comments
Closed
Assignees
Labels
Design Planning Changes still in the thought process Weekly KSv2

Comments

@trjExpensify
Copy link
Contributor

trjExpensify commented Dec 13, 2023

Coming from this convo.

The account settings pages have been moved to the main pane as part of the ideal nav skeleton project, but we didn't update them to the card style design to decouple the implementation from the navigational change. With the feature branch on the cusp of being merged, we're ready to address these changes.

@dannymcclain has gone ahead and redesigned them, so the solution mocks can be found here.

As per @mountiny's suggestion, we'll take this page by page instead of one giant PR for them all. Starting with the profile page which will add the card style to rinse/repeat for the others.

CC: @shawnborton @dannymcclain @JmillsExpensify

@trjExpensify trjExpensify added Weekly KSv2 Planning Changes still in the thought process Design labels Dec 13, 2023
@trjExpensify trjExpensify self-assigned this Dec 13, 2023
Copy link

melvin-bot bot commented Dec 13, 2023

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

@dannymcclain
Copy link
Contributor

Just for supreme clarity, that would be these pages, correct?

Account

  • Profile
  • Wallet
  • Share code
  • Preferences
  • Security
  • Sign out (not really a page I guess)

General

  • Help (also not really a page, right?)
  • About

CleanShot 2023-12-13 at 07 46 04@2x

@trjExpensify
Copy link
Contributor Author

Yah mon, those ones!

@shawnborton
Copy link
Contributor

Also if we need to rejig some of the animations for these (or get new ones) we totally can. I'm guessing now that the cards are going to be a bit wider for things like Preferences, we might need to adjust that animation so the arms are way longer?

@melvin-bot melvin-bot bot added the Overdue label Dec 22, 2023
@dannymcclain
Copy link
Contributor

Not overdue Melvin—just haven't gotten to it yet!

@melvin-bot melvin-bot bot removed the Overdue label Jan 2, 2024
@melvin-bot melvin-bot bot added the Overdue label Jan 10, 2024
@dannymcclain
Copy link
Contributor

Sorry Melv, I think I can get to this soon!

@melvin-bot melvin-bot bot removed the Overdue label Jan 11, 2024
@melvin-bot melvin-bot bot added the Overdue label Jan 19, 2024
@trjExpensify
Copy link
Contributor Author

Skelly is looking pretty good on the latest adhoc build of the ideal nav branch, but it's not on prod yet Melv so this is still technically held.

@melvin-bot melvin-bot bot removed the Overdue label Jan 22, 2024
@JmillsExpensify
Copy link

Woo, looking forward to this! @dannymcclain did you by chance land on the final mocks for this page? Asking because wow the v1 we did for ideal nav is really starting to show it's age. The "card" style looks so so much better.

@dannymcclain
Copy link
Contributor

@JmillsExpensify I've started working on them here, but haven't gotten to all of them or finished polishing quite yet.

@shawnborton
Copy link
Contributor

Oh heck yeah brother, these are looking good already!

@dannymcclain
Copy link
Contributor

@shawnborton one thing I'd like to talk to you and @dubielzyk-expensify about is how our edge to edge option rows work with these new cards. In the past, we've used 20px of left/right padding for everything so them going edge-to-edge wasn't ever an issue.

But now, with varying padding based on viewport (20px mobile, 32px desktop), do they get increased padding on desktop? Or do they stay the same across all viewport sizes?

image

TBH them staying the same across viewports ☝️ doesn't look pad with the way they're currently styled.

I think the issue that arises is more with the push-to-page inputs, which definitely stand out as being not-aligned when they keep their 20px of padding the same on desktop.

image

Feel free to take this convo to Slack or a video chat if that makes more sense. I'm just a little stumped as to how to handle these.

@shawnborton
Copy link
Contributor

I would think we just add an extra 12px of padding to each horizontal side if these components are within a card wrapper and on desktop. Can you share a quick mock of what that might look like? Basically so we can achieve edge-to-edge bleed, but still make it seem like they are aligned perfectly with the text content above them.

@dannymcclain
Copy link
Contributor

Ok that would look like this. I think this is probably the best approach until we revisit the edge-to-edge-ness of all these components 😈

image

image

@shawnborton
Copy link
Contributor

Nice, I agree - let's go with that for now until we revisit all of these. Can't wait until they all look more like the subnav 😈

@dubielzyk-expensify
Copy link
Contributor

Yeah. Let's get to these option row components soon 😅

@dannymcclain
Copy link
Contributor

@dannymcclain
Copy link
Contributor

More updates posted in Slack. Getting close I think.

@dannymcclain
Copy link
Contributor

ALRIGHTY. Final mocks here: https://www.figma.com/file/Xv3yHJooZcxRGkXbAcTZD5/%23ideal-nav-(Ideal-Nav)?type=design&node-id=3125-183278&mode=design&t=XvlzBlUtMgk1sjil-11

We may need to tweak some animations, but personally I think we should start moving forward with implementation and deal with any animation problems as they arise.

Profile
Wallet
Share code
Preferences
Security
About

@mountiny mountiny self-assigned this Jan 31, 2024
@JmillsExpensify JmillsExpensify changed the title [Hold - Ideal Nav Skeleton] Update Account Settings pages to use the new card styles Update Account Settings pages to use the new card styles Jan 31, 2024
@JmillsExpensify
Copy link

Went ahead and removed the hold since we're going to start prioritizing this.

@mountiny
Copy link
Contributor

In touch with SWM and they might get someone to work on this tomorrow using the ideal-nav as a base if it wont be merged to main yet

Just for ease of tracking I propose we split the implementation up to issue per the screen and then we start with the profile page implementation while other pages will be on hold for it (since lots of the card design will be shared).

Once that one page is done and merged, SWM can quickly follow up with the remainder of the pages in parallel. Thoughts?

@JmillsExpensify
Copy link

Agree with going page by page.

@trjExpensify
Copy link
Contributor Author

Yeah, let's have a PR per page 👍

@trjExpensify
Copy link
Contributor Author

Obviously noting that there might be a bit of time inbetween where we have some pages designed a little different, but I think that's okay to avoid a jumbo PR for all of them. 👍

@mountiny
Copy link
Contributor

Yep, I think thats fine and should be resolved in matter of days

@trjExpensify
Copy link
Contributor Author

Yeah, sounds good to me. Updated the OP, now we're ready to rock!

@mountiny
Copy link
Contributor

I will create the issues for this tomorrow and add it to the project

@kosmydel
Copy link
Contributor

kosmydel commented Feb 1, 2024

Hey, I'm from Software Mansion, and I will be responsible for this issue.

Can we consider Figma as the primary source of truth for design changes? If there are any modifications to the design, we should update them in Figma. This approach will simplify our work and eliminate confusion. Thanks!

@filip-solecki
Copy link
Contributor

Hi! I'm Filip from Software Mansion and I will work on this issue too!

@mountiny
Copy link
Contributor

mountiny commented Feb 1, 2024

Can we consider Figma as the primary source of truth for design changes? If there are any modifications to the design, we should update them in Figma. This approach will simplify our work and eliminate confusion. Thanks!

Yeah I think that is the best approach!

I will create the separate issues for each page for y'all today

@kosmydel
Copy link
Contributor

kosmydel commented Feb 1, 2024

Thanks @mountiny! Could you also create a separate issue for handling the icon in the HeaderWithBackButton for all pages?

@hayata-suenaga hayata-suenaga changed the title Update Account Settings pages to use the new card styles [Wave 8] [Ideal Nav] Update Account Settings pages to use the new card styles Feb 2, 2024
@melvin-bot melvin-bot bot added the Overdue label Feb 12, 2024
@trjExpensify
Copy link
Contributor Author

PRs for #35609 & #35603 in review.

@melvin-bot melvin-bot bot removed the Overdue label Feb 12, 2024
@mountiny
Copy link
Contributor

All PRs merged now 🎉

@trjExpensify
Copy link
Contributor Author

Going to close this overall tracker as we have the individual issues for payments etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Planning Changes still in the thought process Weekly KSv2
Projects
No open projects
Development

No branches or pull requests

8 participants