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

Fix logout button appears in two different menus #6230

Merged
merged 3 commits into from
May 7, 2021

Conversation

fzaninotto
Copy link
Member

Closes #6228

@fzaninotto fzaninotto added the RFR Ready For Review label Apr 30, 2021
@@ -89,12 +80,9 @@ const Menu: FC<MenuProps> = props => {
leftIcon={
resource.icon ? <resource.icon /> : <DefaultIcon />
}
onClick={onMenuClick}
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 refactored the <MenuItemLink> to let it pull the props it needs (rather than expecting them to be pushed by the <Sidebar>).

This greatly simplifies the creation of custom menus.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Indeed 👍

docs/Theming.md Outdated Show resolved Hide resolved
@fzaninotto
Copy link
Member Author

Wait, since #5575 it's the menu's responsibility to reduce width when the sidebar is closed. I'm not sure why, but if it's the case this should be documented.

@fzaninotto fzaninotto added WIP Work In Progress and removed RFR Ready For Review labels Apr 30, 2021
@fzaninotto
Copy link
Member Author

OK, it's both the Sidebar AND the Menu's responsibility to reduce width, in order to support one use case: using a react-admin oss <Menu> inside a react-admin ee app. So we need to keep that in the ra oss code, but people who customize the menu in a re oss app don't need to implement open/closed state handling as it's already handled by the Sidebar.

So this PR is ready.

@fzaninotto fzaninotto added the RFR Ready For Review label May 6, 2021
@fzaninotto fzaninotto force-pushed the simplify-menu-customization branch from 086b23f to baa545a Compare May 6, 2021 14:16
@fzaninotto fzaninotto removed the WIP Work In Progress label May 6, 2021
@fzaninotto fzaninotto force-pushed the simplify-menu-customization branch from baa545a to d4256a5 Compare May 6, 2021 14:52
docs/Theming.md Outdated Show resolved Hide resolved
examples/demo/src/layout/Menu.tsx Outdated Show resolved Hide resolved
examples/demo/src/layout/Menu.tsx Outdated Show resolved Hide resolved
@@ -89,12 +80,9 @@ const Menu: FC<MenuProps> = props => {
leftIcon={
resource.icon ? <resource.icon /> : <DefaultIcon />
}
onClick={onMenuClick}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Indeed 👍

@djhi djhi added this to the 3.15.1 milestone May 7, 2021
@djhi djhi merged commit f5645ef into next May 7, 2021
@djhi djhi deleted the simplify-menu-customization branch May 7, 2021 06:52
@fzaninotto fzaninotto modified the milestones: 3.15.1, 3.16.0 May 10, 2021
takayukioda added a commit to takayukioda/react-admin that referenced this pull request Mar 23, 2022
`sidebarIsOpen` in _MenuItemLink_ component and `onMenuClick` in _Menu_ component
has been deprecated in marmelab#6230 and their no longer needed
to create custom menu components.
Also there were some unused import statements, so I've clean them all.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFR Ready For Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants