preactjs-templates / material

The material design `preact-cli` template
MIT License
44 stars 20 forks source link

Uncaught Error: You can't have a focus-trap without at least one focusable element #23

Open shalomvolchok opened 5 years ago

shalomvolchok commented 5 years ago

Navigate to a wildcard profile route like /profile/hello and then open the menu. This throws Uncaught Error: You can't have a focus-trap without at least one focusable element. Then it becomes impossible to close the menu.

This error is coming from the focus-trap package used by MDC. The function is here: https://github.com/davidtheclark/focus-trap/blob/7ea336379d3c292c02fd904564c08d8370ecb1b5/index.js#L158

If we set selected on a route, then it works. But in the case where we have a route that doesn't have a menu item nothing gets selected and we get the focus trap error. One option is adding tabindex, which also makes the menu work with tabs and enter. I'm doing something like this in a project right now. I can make a PR if you'd like to add it into this template.

<Drawer modal ref={this.drawerRef}>
    <Drawer.DrawerContent>
        <Drawer.DrawerItem
            selected={props.selectedRoute === "/"}
            onClick={this.goHome}
            onKeyDown={({ keyCode }) => {
                keyCode === 13 && this.goHome();
            }}
            tabindex={0}
            role="link"
        >
            <List.ItemGraphic>home</List.ItemGraphic>
            Home
        </Drawer.DrawerItem>
        <Drawer.DrawerItem
            selected={props.selectedRoute === "/profile"}
            onClick={this.goToMyProfile}
            onKeyDown={({ keyCode }) => {
                keyCode === 13 && this.goToMyProfile();
            }}
            tabindex={0}
            role="link"
        >
            <List.ItemGraphic>account_circle</List.ItemGraphic>
            Profile
        </Drawer.DrawerItem>
    </Drawer.DrawerContent>
</Drawer>

Or if we don't care about tabbing we can just wrap the whole thing in an element with tabindex={0}, but that probably breaks accessibility.

<Drawer.DrawerContent>
    <div class={style.menuFocusTrap} tabindex={0}>
        <Drawer.DrawerItem/>
        <Drawer.DrawerItem/>
    </div>
</Drawer.DrawerContent>
.menuFocusTrap {
    border: 0;
    outline: 0;
}