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

feat: create Drawer component #4475

Merged
merged 100 commits into from
Jan 22, 2025
Merged

feat: create Drawer component #4475

merged 100 commits into from
Jan 22, 2025

Conversation

adrians5j
Copy link
Member

@adrians5j adrians5j commented Jan 7, 2025

Changes

This PR creates the Drawer component. Here is a pair of sshots.

The first is File Manager, where modal prop is set to false (default value actually). This basically means the user can also interact with the UI in the background and that the drawer is only closed when pressing Esc or clicking on the X icon. This was done as discussed with Kreso.

image

This is the main menu, where modal prop is set to true, meaning no background interactions are possible.

image

Another random sshot:

image

Extra Changes

1. Updated File Manager Drawer Code

Had to accommodate the file details drawer in File Manager a bit.

Did some cleanups along the way too (some styled components are no longer needed).

2. File Manager - Increased Size of Buttons

I made these buttons larger:

image

3. Updated Query Builder Drawer Code

Had to accommodate the file details drawer in ACO's Query Builder a bit.

Did some cleanups along the way too (some styled components are no longer needed).

Notes

1. Tabs in Dialog

We had a discussion on tabs being rendered in an opened dialog. There were a couple of options here, but, ultimately, I've done it in a way where you pass bodyPadding: false prop to the Dialog component. That way, the body has no padding, and tabs can be used within the dialog as imagined in Figma designs (they taking the full width of the dialog).

@leopuleo
Copy link
Contributor

The PR looks lovely, thank you Adrian 🙏
I left some comments here and there, and these are minor change requests.

I have a couple of general comments:

1) Scrollable content

The content inside the drawer is not scrollable. Check out the video:

CleanShot.2025-01-22.at.09.09.28.mp4

Shall we try to prevent this behaviour by default or leave the developer to handle it when needed?

2) Internal components naming

I've noticed that the internal components have the Drawer prefix. I usually don't use it, so DrawerBody -> Body.

It's debatable, and after we finish developing all the components, we should go through the library and adjust these minor aspects.

3) Use of children as Drawer prop

Similar to the Dialog component, I've noticed we are using the default children prop to provide the content to the drawer. Shall we use dedicated props like content, similar to what we do with title, description etc.?

Also in this case, we can adjust this later.

…into feat/drawer

# Conflicts:
#	packages/admin-ui/scripts/importFromFigma/exports/Alias tokens.json
#	packages/admin-ui/src/Dialog/components/DialogContent.tsx
#	packages/admin-ui/src/Dialog/components/DialogFooter.tsx
#	packages/admin-ui/src/DropdownMenu/DropdownMenu.tsx
#	packages/ui/package.json
#	yarn.lock
@adrians5j
Copy link
Member Author

Resolved scrollable content issue by adding overflow-auto to the drawer body section.

So, scrollbars will show if the height of the body is not enough to show all content in it. The main menu is also showing scrollbars now.

Re other points... we can discuss them later.

@adrians5j adrians5j changed the base branch from feat/dialog to feat/new-admin-ui January 22, 2025 13:53
…into feat/drawer

# Conflicts:
#	packages/admin-ui/src/index.ts
#	packages/ui/package.json
#	yarn.lock
@adrians5j adrians5j changed the title New Admin UI - Drawer feat: create Drawer component Jan 22, 2025
@adrians5j adrians5j merged commit 4760ad2 into feat/new-admin-ui Jan 22, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants