Skip to content

[website] Improve Base UI hero section demo#38255

Merged
danilo-leal merged 12 commits intomasterfrom
base-ui-page-improvements
Aug 14, 2023
Merged

[website] Improve Base UI hero section demo#38255
danilo-leal merged 12 commits intomasterfrom
base-ui-page-improvements

Conversation

@danilo-leal
Copy link
Collaborator

@danilo-leal danilo-leal commented Jul 31, 2023

Following @oliviertassinari's feedback on the original page PR (#36622 (comment)). This PR is limiting itself, for the sake of scope, to the hero section demo only. There are other page improvements reported there but those will happen in a different one.

  • Lighter text color on the "View snackbar" and "View all components" buttons when in dark mode.
  • Making the Select's label wired to the component + triggering the button.
  • Fix the Switch focus style.
  • Make the Switch accompanying text a label element rather than p.
  • Add a consistent focus visible treatment to all components.
  • Scrollbar layout shift after the Modal's transition.
  • Fix the avatar's missing image.
  • Enable the Escape key to close the modal (covered in [website] Base UI Modal can't be closed with the Escape key #38468).

https://deploy-preview-38255--material-ui.netlify.app/base-ui/


The items below will come later once an overarching animation strategy for Base is figured out (#38280).

  • Add a transition to the Select list box + menu dropdown.
  • Add a transition to the Slider component.

@danilo-leal danilo-leal added package: @mui/base Specific to @mui/base (legacy). website Pages that are not documentation-related, marketing-focused. labels Jul 31, 2023
@danilo-leal danilo-leal added this to the Base UI: Stable release milestone Jul 31, 2023
@danilo-leal danilo-leal self-assigned this Jul 31, 2023
@danilo-leal
Copy link
Collaborator Author

danilo-leal commented Jul 31, 2023

@michaldudak & @oliviertassinari hey y'all! I'll need your help with some of the unchecked items:

Enable the Escape key to close the modal.
From what I've played with here, that's not happening because the Modal has the disablePortal prop. When I take that off it works well but the styles are then hurt (border-radius + borders are gone).

Scrollbar layout shift after the Modal's transition.
Couldn't reproduce this one and also have no clue how to tackle it. 😬

Add a transition to the Select list box + menu dropdown
How can I do this without an exposed open prop similar to how the Modal has?

Add a transition to the Slider component
None of the Slider demos on the Base UI docs have this transition (commented here #36622 (comment)). I'd love to have it but is there a way to do it without relying on any Material UI transition components?

@michaldudak
Copy link
Member

Enable the Escape key to close the modal.
From what I've played with here, that's not happening because the Modal has the disablePortal prop

Weird, whether the Modal uses portals or not shouldn't affect the behavior of the Escape key. I'll look into it.

Scrollbar layout shift after the Modal's transition.

This is apparent on Windows as the scrollbar disappears whenever the Modal is shown, causing a layout shift. As I use Windows, I can tackle it as well.

Add a transition to the Select list box + menu dropdown
Add a transition to the Slider component

We'll have to wait until #38280 is done for these.

@danilo-leal
Copy link
Collaborator Author

Sweet, thanks for the help! I think we can merge these improvements without the animation. It'd definitely be great to have it but I don't want to use Material UI components for that to make it as pure as possible!

@mui-bot
Copy link

mui-bot commented Aug 2, 2023

Netlify deploy preview

https://deploy-preview-38255--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against ada25bd

@michaldudak
Copy link
Member

I fixed the layout shift, but the inability to close the modal with the Escape key is more tricky. It may be a subtle bug within the Modal or Focus Trap, as the modal body doesn't get focused after it's open and does not respond to keyboard events. I'll spend some more time on this tomorrow.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Aug 3, 2023
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Aug 4, 2023
Copy link
Member

@michaldudak michaldudak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's merge it in its current state and create an issue with the Escape key not working (#38468)

@danilo-leal danilo-leal merged commit b426997 into master Aug 14, 2023
@danilo-leal danilo-leal deleted the base-ui-page-improvements branch August 14, 2023 11:48
@oliviertassinari
Copy link
Member

One regression I noticed:

Before

Screenshot 2023-08-14 at 19 50 10

After

Screenshot 2023-08-14 at 19 49 10

@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 14, 2023

The modal animation feel so much better with a stable layout, how did you solve it?

I have updated #36622 (comment) with checkboxes to keep track of what's done for clarity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package: @mui/base Specific to @mui/base (legacy). website Pages that are not documentation-related, marketing-focused.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants