New sidemenu for notes editor #6527
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR introduces a custom side menu for the block editor, enhancing the user experience with new components and styles.
- New Custom Side Menu: Added
CustomSideMenu.tsxto provide block editing options. - Block Addition Component: Introduced
CustomAddBlockItem.tsxfor adding new blocks. - Side Menu Options: Added
CustomSideMenuOptions.tsxfor menu item styling. - Editor Styling: Updated
BlockEditor.tsxwith new styles for better UI consistency. - Layout Adjustment: Modified
ShowPageActivityContainer.tsxto increase top margin for improved layout.
6 file(s) reviewed, 7 comment(s)
Edit PR Review Bot Settings
| }; | ||
| return ( | ||
| <button | ||
| className="mantine-focus-auto m_99ac2aa1 mantine-Menu-item bn-menu-item m_87cf2631 mantine-UnstyledButton-root" |
There was a problem hiding this comment.
style: Consider using a more descriptive class name instead of mantine-focus-auto.
There was a problem hiding this comment.
Seems strange to have m_99ac2aa1 or m_87cf2631, are you sure we need those?
There was a problem hiding this comment.
Will get back to you about this with some more clarity
There was a problem hiding this comment.
Hey @FelixMalfait,
I checked the classes m_99ac2aa1 and m_87cf2631 in the code. They’re handling specific styles like padding, cursor behavior, and other appearance settings using Mantine’s spacing and color variables.
If we want to avoid using these Mantine utility classes, I can replicate the styles manually. The only thing to consider would be how to handle Mantine’s spacing variables like --mantine-spacing-xs and --mantine-spacing-sm ourselves.
Let me know if you prefer that approach, or if you’re okay with keeping these classes for now.
| // eslint-disable-next-line react/jsx-props-no-spreading | ||
| <SideMenu {...props}> |
There was a problem hiding this comment.
style: Consider removing the eslint-disable comment by refactoring to avoid prop spreading.
| // eslint-disable-next-line react/jsx-props-no-spreading | ||
| {...props} |
There was a problem hiding this comment.
style: Avoid prop spreading to improve maintainability and readability.
| // eslint-disable-next-line react/jsx-props-no-spreading | ||
| <DragHandleMenu {...props}> |
There was a problem hiding this comment.
style: Refactor to avoid prop spreading for better code quality.
| {/* eslint-disable-next-line react/jsx-props-no-spreading */} | ||
| <BlockColorsItem {...props}> |
There was a problem hiding this comment.
style: Prop spreading should be avoided to prevent potential bugs.
| {/* eslint-disable-next-line react/jsx-props-no-spreading */} | ||
| <RemoveBlockItem {...props}> |
There was a problem hiding this comment.
style: Avoid using prop spreading to enhance code clarity.
packages/twenty-front/src/modules/ui/input/editor/components/CustomSideMenuOptions.tsx
Outdated
Show resolved
Hide resolved
packages/twenty-front/src/modules/ui/input/editor/components/CustomSideMenuOptions.tsx
Outdated
Show resolved
Hide resolved
packages/twenty-front/src/modules/ui/input/editor/components/CustomSideMenuOptions.tsx
Outdated
Show resolved
Hide resolved
| className="mantine-focus-auto m_99ac2aa1 mantine-Menu-item bn-menu-item m_87cf2631 mantine-UnstyledButton-root" | ||
| onClick={handleClick} | ||
| > | ||
| <Components.Generic.Menu.Item onClick={handleClick}> |
There was a problem hiding this comment.
That makes so much more sense. haha mb
Log |
| editor, | ||
| children, | ||
| }: CustomAddBlockItemProps) => { | ||
| const Components = useComponentsContext(); |
There was a problem hiding this comment.
@FelixMalfait
this should've been const Components = useComponentsContext()! to remove ts errors.
There was a problem hiding this comment.
Ah damn, I don't really like the const Components = useComponentsContext()! but should have done an if() + return to avoid the scenario where it's null. Would you mind raising a PR for this? Otherwise I will do it today! Thanks a lot!
There was a problem hiding this comment.
I will raise it dw. Will also remove some of the entityId ts errors. ps: there are 3 of them
Will clean it up
@Bonapara @lucasbordeau
This PR addresses issue #6489:
Review Request
Please review the implementation of the custom sidemenu.
Outstanding Issues
Sidemenu Positioning:
Props Spreading in
CustomSidemenu.tsx:Your insights on these challenges would be greatly appreciated, especially regarding the sidemenu positioning and any potential alternatives to props spreading.
2024-08-04.14-32-37.mp4