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

[docs] Mobile nav #881

Merged
merged 26 commits into from
Nov 28, 2024
Merged

[docs] Mobile nav #881

merged 26 commits into from
Nov 28, 2024

Conversation

vladmoroz
Copy link
Contributor

@vladmoroz vladmoroz commented Nov 27, 2024

@vladmoroz vladmoroz added the docs Improvements or additions to the documentation label Nov 27, 2024
@vladmoroz vladmoroz requested a review from colmtuite November 27, 2024 17:57
@mui-bot
Copy link

mui-bot commented Nov 27, 2024

Netlify deploy preview

https://deploy-preview-881--base-ui.netlify.app/

Generated by 🚫 dangerJS against 408f0ee

@colmtuite
Copy link
Contributor

@vladmoroz It's exceptionally good overall 👏

  1. A few times I accidentally selected text on my iphone. Set user-select: none on the sheet navigation links?
  2. When you open the navigation dialog in a small viewport, then resize to a large viewport, the dialog remains visible.
  3. Are the links too small? Afaik, these kinds of nav menu links are usually a little larger on mobile. I checked FB and a few other apps.
  4. Are the separators between links a tiny bit too high contrast? It's more noticeable in dark mode. But I think I'd reduce contrast a tiny bit in both color modes.
  5. The sheet animation feels too slow to me. I might make it ~50% faster.

@vladmoroz
Copy link
Contributor Author

vladmoroz commented Nov 27, 2024

When you open the navigation dialog in a small viewport, then resize to a large viewport, the dialog remains visible.

Yeah I had used to close mobile menus when resizing to a larger viewport. I considered it here and decided that it's unwanted motion and it doesn't feel solid. As a user, if I did end up in this situation legitimately, I think I'd rather keep seeing the open menu. Switching context with a different page layout feels too tasking; it's easier to pick up where you left off and finish the navigation.

Are the separators between links a tiny bit too high contrast? It's more noticeable in dark mode. But I think I'd reduce contrast a tiny bit in both color modes.

Do you feel the same about the main content separators? They use the same colour

The sheet animation feels too slow to me. I might make it ~50% faster.

I know your taste with animations, but if anything, I'd say it's already a bit too fast. It might feel a bit slow on desktop, but seems right or on the fast side on the actual device. Native iOS sheets that are about twice as short as this one transition about 30-50% slower.

Also I hope you don't look at the actual ms value as I'm using an extremely steep curve for the timing function. ease or ease-out would feel much slower.


A few times I accidentally selected text on my iphone. Set user-select: none on the sheet navigation links?

Will check. iirc user-select: none is wonky on mobile but I'll try.

Are the links too small? Afaik, these kinds of nav menu links are usually a little larger on mobile. I checked FB and a few other apps.

Will give a larger size a go

@colmtuite
Copy link
Contributor

I considered it here and decided that it's unwanted motion

Ok.

Do you feel the same about the main content separators?

Yep, too high contrast for me.

animations

I did most of my testing on my iPhone 12 mini. I checked a few more apps since and I'm finding a mixed bag of timings. Deliveroo iOS seems similar in duration to your animation. Perhaps it's fine.

Will give a larger size a go

Deliveroo iOS sheet links also seem the same size? Perhaps this is also a matter of taste + user demographic.

@colmtuite
Copy link
Contributor

@vladmoroz Bug:

  1. Load page in iOS.
  2. Open sheet navigation.
  3. Swipe to dismiss.
  4. Window scroll is broken.

@vladmoroz
Copy link
Contributor Author

@vladmoroz Bug:

  1. Load page in iOS.
  2. Open sheet navigation.
  3. Swipe to dismiss.
  4. Window scroll is broken.

@colmtuite yeah it's a bug in popups, there's a fix coming in #878

@vladmoroz
Copy link
Contributor Author

A few times I accidentally selected text on my iphone. Set user-select: none on the sheet navigation links?

@colmtuite new Safari bug 🫠
With user-select: none, the link text disappears on long press

RPReplay_Final1732796404.mov

Without user-select: none

RPReplay_Final1732796382.mov

Looks like that's ruled out

@vladmoroz vladmoroz merged commit 192a8a9 into mui:master Nov 28, 2024
21 of 22 checks passed
@vladmoroz vladmoroz deleted the hamburger branch November 28, 2024 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants