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

UserMenu example does not link to route #8409

Closed
nkappler opened this issue Nov 20, 2022 · 4 comments · Fixed by #8678
Closed

UserMenu example does not link to route #8409

nkappler opened this issue Nov 20, 2022 · 4 comments · Fixed by #8678

Comments

@nkappler
Copy link
Contributor

nkappler commented Nov 20, 2022

My Code Code Example from UserMenu.tsx
const MyProfile = 
React.forwardRef((props, ref) => {
    const { onClose } = useUserMenu();
    return 
    < MenuItem
       ref={ref} // ignored in "Upgrading to v4" guide
       {...props}
       // used instead in "Upgrading to v4" guide:
    // component={Link}
       to="/profile"
       onClick={onClose}
    >
        < ListItemIcon>
            < SettingsIcon />
        < /ListItemIcon>
        < ListItemText >
            My Profile
        < /ListItemText>
    < /MenuItem>;
});

const UserMenu = (props: UserMenuProps) => 
< RaUserMenu {...props}>
    < MyProfile />
    < Logout />
< /RaUserMenu >;
const ConfigurationMenu = 
React.forwardRef((props, ref) => {
    const { onClose } = useUserMenu();
    return (
        < MenuItem
            ref={ref}
            {...props}


            to="/configuration"
            onClick={onClose}
        >
            < ListItemIcon>
                < SettingsIcon />
            < /ListItemIcon>
            < ListItemText>
                Configuration
            < /ListItemText>
        < /MenuItem>);
});

const MyUserMenu = () => (
    < UserMenu>
        < ConfigurationMenu />
        < Logout />
    < /UserMenu>
);

Above you can see my code compared to the example given by react-admin. (The additional spaces at the beginning of tags are only there so the markdown will render correctly in a table)
As you can see it is more or less identical and it also renders correctly (apart from inconsistent spacing):

grafik

However, when I click on My Profile, nothing happens, except that the menu closes.
I'd expect the navigation to happen, since it is never mentioned in the example you would have to implement it manually.

In the Upgrading to v4 Guide there is another example for this, but using the component property with Link and ignoring the ref property. (You can see it as a comment in my code above).
With this variant the navigation will work, but there are now runtime errors because ref is ignored.

Another issue I found is that the menu item will always look like it is active. With MenuItemLink this would work and only highlight the item when the corresponding Route is active.

  • I can't get this to work without errors and both documentation examples fall short.
  • What is the recommended way to do this?
  • The documentation should get updated accordingly
@nkappler
Copy link
Contributor Author

StackBlitz Demo

@nkappler
Copy link
Contributor Author

The errors I face when using the example from the Upgrading to v4 Guide:
Warning: Failed prop type: Invalid prop `component` supplied to `ForwardRef(ButtonBase)`. Expected an element type that can hold a ref. Did you accidentally provide a plain function component instead? For more information see https://mui.com/r/caveat-with-refs-guide
ButtonBase@http://localhost:3000/static/js/bundle.js:18639:82
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
MenuItem@http://localhost:3000/static/js/bundle.js:31633:83
./src/layout/Layout.tsx/_c<@http://localhost:3000/static/js/bundle.js:8565:63
ul
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
List@http://localhost:3000/static/js/bundle.js:31344:82
MenuList@http://localhost:3000/static/js/bundle.js:31907:9
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
div
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
Paper@http://localhost:3000/static/js/bundle.js:34385:82
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
Paper@http://localhost:3000/static/js/bundle.js:34385:82
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
Transition@http://localhost:3000/static/js/bundle.js:179203:30
Grow@http://localhost:3000/static/js/bundle.js:27020:9
FocusTrap@http://localhost:3000/static/js/bundle.js:12482:7
div
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
Portal@http://localhost:3000/static/js/bundle.js:13821:7
ModalUnstyled@http://localhost:3000/static/js/bundle.js:13054:9
Modal@http://localhost:3000/static/js/bundle.js:32495:82
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
Popover@http://localhost:3000/static/js/bundle.js:34615:83
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
Menu@http://localhost:3000/static/js/bundle.js:32176:83
UserMenuContextProvider@http://localhost:3000/static/js/bundle.js:144347:18
div
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
UserMenu@http://localhost:3000/static/js/bundle.js:144171:59
UserMenu
div
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
Toolbar@http://localhost:3000/static/js/bundle.js:41923:82
header
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
Paper@http://localhost:3000/static/js/bundle.js:34385:82
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
AppBar@http://localhost:3000/static/js/bundle.js:16320:83
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
Transition@http://localhost:3000/static/js/bundle.js:179203:30
Slide@http://localhost:3000/static/js/bundle.js:36504:77
HideOnScroll@http://localhost:3000/static/js/bundle.js:142357:18
./node_modules/ra-ui-materialui/dist/esm/layout/AppBar.js/AppBar<@http://localhost:3000/static/js/bundle.js:141844:18
AppBar
div
div
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
Layout@http://localhost:3000/static/js/bundle.js:142428:12
Layout@http://localhost:3000/static/js/bundle.js:8635:7
div
RenderedRoute@http://localhost:3000/static/js/bundle.js:178422:7
Routes@http://localhost:3000/static/js/bundle.js:178843:7
CoreAdminRoutes@http://localhost:3000/static/js/bundle.js:121655:77
RenderedRoute@http://localhost:3000/static/js/bundle.js:178422:7
Routes@http://localhost:3000/static/js/bundle.js:178843:7
CoreAdminUI@http://localhost:3000/static/js/bundle.js:121761:12
div
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
ScopedCssBaseline@http://localhost:3000/static/js/bundle.js:35303:82
AdminUI@http://localhost:3000/static/js/bundle.js:131811:22
InnerThemeProvider@http://localhost:3000/static/js/bundle.js:46881:70
ThemeProvider@http://localhost:3000/static/js/bundle.js:45786:7
ThemeProvider@http://localhost:3000/static/js/bundle.js:46902:7
ThemeProvider@http://localhost:3000/static/js/bundle.js:143914:18
ResourceDefinitionContextProvider@http://localhost:3000/static/js/bundle.js:122044:12
NotificationContextProvider@http://localhost:3000/static/js/bundle.js:129075:18
I18nContextProvider@http://localhost:3000/static/js/bundle.js:128108:12
Router@http://localhost:3000/static/js/bundle.js:178785:7
HistoryRouter@http://localhost:3000/static/js/bundle.js:129621:18
InternalRouter@http://localhost:3000/static/js/bundle.js:129538:18
BasenameContextProvider@http://localhost:3000/static/js/bundle.js:129591:18
AdminRouter@http://localhost:3000/static/js/bundle.js:129520:17
QueryClientProvider@http://localhost:3000/static/js/bundle.js:175596:16
PreferencesEditorContextProvider@http://localhost:3000/static/js/bundle.js:129277:18
StoreContextProvider@http://localhost:3000/static/js/bundle.js:129955:15
CoreAdminContext@http://localhost:3000/static/js/bundle.js:121586:22
AdminContext@http://localhost:3000/static/js/bundle.js:131755:12
Admin@http://localhost:3000/static/js/bundle.js:149095:22
App react.development.js:220
Warning: Function components cannot be given refs. Attempts to access this ref will fail. Did you mean to use React.forwardRef()?

Check the render method of MuiButtonBaseRoot.
Link@http://localhost:3000/static/js/bundle.js:131988:12
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
ButtonBase@http://localhost:3000/static/js/bundle.js:18639:82
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
MenuItem@http://localhost:3000/static/js/bundle.js:31633:83
./src/layout/Layout.tsx/_c<@http://localhost:3000/static/js/bundle.js:8565:63
ul
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
List@http://localhost:3000/static/js/bundle.js:31344:82
MenuList@http://localhost:3000/static/js/bundle.js:31907:9
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
div
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
Paper@http://localhost:3000/static/js/bundle.js:34385:82
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
Paper@http://localhost:3000/static/js/bundle.js:34385:82
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
Transition@http://localhost:3000/static/js/bundle.js:179203:30
Grow@http://localhost:3000/static/js/bundle.js:27020:9
FocusTrap@http://localhost:3000/static/js/bundle.js:12482:7
div
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
Portal@http://localhost:3000/static/js/bundle.js:13821:7
ModalUnstyled@http://localhost:3000/static/js/bundle.js:13054:9
Modal@http://localhost:3000/static/js/bundle.js:32495:82
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
Popover@http://localhost:3000/static/js/bundle.js:34615:83
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
Menu@http://localhost:3000/static/js/bundle.js:32176:83
UserMenuContextProvider@http://localhost:3000/static/js/bundle.js:144347:18
div
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
UserMenu@http://localhost:3000/static/js/bundle.js:144171:59
UserMenu
div
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
Toolbar@http://localhost:3000/static/js/bundle.js:41923:82
header
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
Paper@http://localhost:3000/static/js/bundle.js:34385:82
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
AppBar@http://localhost:3000/static/js/bundle.js:16320:83
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
Transition@http://localhost:3000/static/js/bundle.js:179203:30
Slide@http://localhost:3000/static/js/bundle.js:36504:77
HideOnScroll@http://localhost:3000/static/js/bundle.js:142357:18
./node_modules/ra-ui-materialui/dist/esm/layout/AppBar.js/AppBar<@http://localhost:3000/static/js/bundle.js:141844:18
AppBar
div
div
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
Layout@http://localhost:3000/static/js/bundle.js:142428:12
Layout@http://localhost:3000/static/js/bundle.js:8635:7
div
RenderedRoute@http://localhost:3000/static/js/bundle.js:178422:7
Routes@http://localhost:3000/static/js/bundle.js:178843:7
CoreAdminRoutes@http://localhost:3000/static/js/bundle.js:121655:77
RenderedRoute@http://localhost:3000/static/js/bundle.js:178422:7
Routes@http://localhost:3000/static/js/bundle.js:178843:7
CoreAdminUI@http://localhost:3000/static/js/bundle.js:121761:12
div
./node_modules/@emotion/react/dist/emotion-element-6a883da9.browser.esm.js/withEmotionCache/<@http://localhost:3000/static/js/bundle.js:9648:66
ScopedCssBaseline@http://localhost:3000/static/js/bundle.js:35303:82
AdminUI@http://localhost:3000/static/js/bundle.js:131811:22
InnerThemeProvider@http://localhost:3000/static/js/bundle.js:46881:70
ThemeProvider@http://localhost:3000/static/js/bundle.js:45786:7
ThemeProvider@http://localhost:3000/static/js/bundle.js:46902:7
ThemeProvider@http://localhost:3000/static/js/bundle.js:143914:18
ResourceDefinitionContextProvider@http://localhost:3000/static/js/bundle.js:122044:12
NotificationContextProvider@http://localhost:3000/static/js/bundle.js:129075:18
I18nContextProvider@http://localhost:3000/static/js/bundle.js:128108:12
Router@http://localhost:3000/static/js/bundle.js:178785:7
HistoryRouter@http://localhost:3000/static/js/bundle.js:129621:18
InternalRouter@http://localhost:3000/static/js/bundle.js:129538:18
BasenameContextProvider@http://localhost:3000/static/js/bundle.js:129591:18
AdminRouter@http://localhost:3000/static/js/bundle.js:129520:17
QueryClientProvider@http://localhost:3000/static/js/bundle.js:175596:16
PreferencesEditorContextProvider@http://localhost:3000/static/js/bundle.js:129277:18
StoreContextProvider@http://localhost:3000/static/js/bundle.js:129955:15
CoreAdminContext@http://localhost:3000/static/js/bundle.js:121586:22
AdminContext@http://localhost:3000/static/js/bundle.js:131755:12
Admin@http://localhost:3000/static/js/bundle.js:149095:22
App react-dom.development.js:67
MUI: Unable to set focus to a MenuItem whose component has not been rendered. MenuItem.js:153

@nkappler
Copy link
Contributor Author

The only way I found where the Link works and there are no errors is by wrapping the Link component in an extra div with forward Ref.
This however breaks MUI's keyboard navigation inside the menu and the styling is still broken.

const MyLink = React.forwardRef((props: any, ref: any) => <div ref={ref}><Link {...props} /></div>);

const MyProfile = (props: any) => {

    const { onClose } = useUserMenu();

    return <MenuItem
        {...props}
        component={MyLink}
        to="/profile"
        onClick={onClose}
    >
        <ListItemIcon>
            <SettingsIcon />
        </ListItemIcon>
        <ListItemText >
            My Profile
        </ListItemText>
    </MenuItem>;
};

@WiXSL
Copy link
Contributor

WiXSL commented Nov 21, 2022

Thanks for the report.
We have to fix this part of the upgrade guide

Check on the demo project example.

const ConfigurationMenu = React.forwardRef((props, ref) => {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants