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

Created a reusable "Drawer" component #89

Merged
merged 7 commits into from
Dec 14, 2023

Conversation

nick-y-ito
Copy link
Member

@nick-y-ito nick-y-ito commented Dec 14, 2023

Overviews

Created a reusable "Drawer" component and its usage example file.

Review Points

  • Focused on designing the drawer of the image below
image
  • Remove styling the green background drawers from the scope of this PR
Screenshot 2023-12-13 at 16 21 52

Screenshots

  • The body is scrollable when the content exceeds the parent height
  • The overlay remains of "2rem"
Untitled.mov
  • If the body height is lower than its parent, the height of the entire drawer shrinks to fit the content
Screen.Recording.2023-12-13.at.15.59.36.mov
  • The Drawer can be set to the right side
  • The size of the right drawer may need to be adjusted when applying it
Screen.Recording.2023-12-13.at.16.04.24.mov

@nick-y-ito nick-y-ito linked an issue Dec 14, 2023 that may be closed by this pull request
2 tasks
@nick-y-ito nick-y-ito marked this pull request as ready for review December 14, 2023 00:56
@kanta1207
Copy link
Member

kanta1207 commented Dec 14, 2023

LGTM!
Tried to find a room for improvement but couldn't!
Beautifully implemented 👏

I'll approve it, but in case you can of course wait for Kota if you want

ComponentPropsWithoutRef<typeof PrimitiveOverlay>
>(({ className, ...props }, ref) => (
<PrimitiveOverlay
className={cn(
Copy link
Member

Choose a reason for hiding this comment

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

I liked how you classify className by it's categories!
Maybe we can create a note or doc somewhere about how we classify and organize classNames in cn() method

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a good idea!
I was actually considering creating a STYLEGUIDE.md file to document coding rules for a project, such as using I to name interfaces like IMyProps.

Shall we create a new issue for that?

Copy link
Member Author

Choose a reason for hiding this comment

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

Created #91

@nick-y-ito
Copy link
Member Author

nick-y-ito commented Dec 14, 2023

@kanta1207
Thank you for your review! I will wait for @kotaaaa 's review, just in case.

/**
* This is a custom component that we use to wrap the body (between header and footer) of the dialog.
*/
const DialogBody = forwardRef<ElementRef<'div'>, HTMLAttributes<HTMLDivElement>>(
Copy link
Member

Choose a reason for hiding this comment

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

Do you still need this DialogBody?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, Yes, don't care about this comment

Copy link
Member Author

Choose a reason for hiding this comment

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

😆

Copy link
Member

@kotaaaa kotaaaa left a comment

Choose a reason for hiding this comment

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

Thank you for implementation!!!

@nick-y-ito nick-y-ito merged commit 423e492 into develop Dec 14, 2023
2 checks passed
@nick-y-ito nick-y-ito deleted the feature/58-drawer-component branch December 14, 2023 02:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create "Drawer" shared UI component
3 participants