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

[joy-ui][Drawer] Add Drawer component #38169

Merged
merged 59 commits into from
Sep 8, 2023

Conversation

mnajdova
Copy link
Member

@mnajdova mnajdova commented Jul 26, 2023

closes #36292

Adding Drawer related components in Joy UI. I started with adding a Drawer component - which is the equivalent of the temporary drawer in Material UI. I decided not to add the permanent variant version, because in my opinion, these two versions of the component are too different to justify them being bundled in one component - if developers need a permanent drawer, why bundle all the modal-related code with it?

After this PR:

  • Add PermanentDrawer (Sidebar) component - we should have an integration demo about how the two can be used together.
  • Add SwipeableDrawer component
  • Add PersistentDrawer component

@mnajdova mnajdova added component: drawer This is the name of the generic UI component, not the React module! package: joy-ui Specific to @mui/joy labels Jul 26, 2023
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 26, 2023
@mui-bot
Copy link

mui-bot commented Jul 26, 2023

Netlify deploy preview

@mui/joy: parsed: +1.17% , gzip: +1.17%

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 84208ea

@siriwatknp

This comment was marked as resolved.

@mnajdova

This comment was marked as resolved.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 27, 2023
@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 16, 2023
@@ -3,6 +3,8 @@ import { generateUtilityClass, generateUtilityClasses } from '../className';
export interface ModalClasses {
/** Class name applied to the root element. */
root: string;
/** Class name applied to the root element when open is false. */
hidden: string;
Copy link
Member Author

Choose a reason for hiding this comment

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

Missing class name

@zanivan
Copy link
Contributor

zanivan commented Aug 31, 2023

There are some items that might be worth mentioning/discussing:

  • The variants and colors are not being applied to the playground demo
  • When switching modes, the iframe background sometimes appears and sometimes not
Light Dark
Screenshot 2023-08-31 at 18 37 07 Screenshot 2023-08-31 at 18 38 19
  • It's impossible to dismiss the lg modal on mobile
Screenshot

I'm still getting up to speed on this, and you might already be aware of these things, so sorry about that.
Just to let you know, I've started refining the demos designs. Should I wait a bit more, or keep working on them?

The Drawer can be cancelled by clicking the overlay or pressing the Esc key.
It closes when an item is selected, handled by controlling the `open` prop.

{{"demo": "DrawerBasic.js", "iframe": true}}
Copy link
Member

Choose a reason for hiding this comment

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

Does it need to be iframe given that other demos are not? cc @zanivan

Copy link
Member Author

Choose a reason for hiding this comment

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

I will change this demo to be similar to the others, I wanted to make it closer to showcase how it would look like in app, but it complicates things in the code itself - the usage of the document object etc.

@mnajdova
Copy link
Member Author

mnajdova commented Sep 6, 2023

Sorry for the delay. Here are some answers:

The variants and colors are not being applied to the playground demo

Fixed, I must have broken this when I was removing the DrawerContent component.

When switching modes, the iframe background sometimes appears and sometimes not

I simplified this demo, no need for iframe, it complicates things when exporting to sandbox too.

It's impossible to dismiss the lg modal on mobile

It's possible by clicking on some of the items. In practise, I wouldn't expect that someone would use large drawer on a mobile?

@mnajdova mnajdova requested review from siriwatknp and zanivan and removed request for zanivan September 6, 2023 07:19
Copy link
Contributor

@zanivan zanivan left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@siriwatknp
Copy link
Member

siriwatknp commented Sep 8, 2023

It's impossible to dismiss the lg modal on mobile

It's possible by clicking on some of the items. In practise, I wouldn't expect that someone would use large drawer on a mobile?

I added support for this by setting the width to min(100vw, drawer-width). With this, the drawer's width will never exceed the viewport's width which is commonly expected.

Copy link
Member

@siriwatknp siriwatknp left a comment

Choose a reason for hiding this comment

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

🚀🚀🚀

@mnajdova mnajdova merged commit 3d2fb8a into mui:master Sep 8, 2023
21 checks passed
@marcpachecog
Copy link

marcpachecog commented Sep 8, 2023

Thanks @siriwatknp , @mnajdova and @zanivan for your hard and meticulous work!

@Usama-Tahir
Copy link

Great work. I have a question. When will it be available via npm?

Thanks

anon-phantom pushed a commit to anon-phantom/material-ui that referenced this pull request Sep 11, 2023
@mnajdova
Copy link
Member Author

Great work. I have a question. When will it be available via npm?

Likely later today with the latest release :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: drawer This is the name of the generic UI component, not the React module! package: joy-ui Specific to @mui/joy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Joy] Add the Drawer component
6 participants