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

SideNav and overlay behaviour on click #3666

Closed
2 tasks done
nxn-4-wdf opened this issue Aug 7, 2019 · 13 comments · Fixed by #8296
Closed
2 tasks done

SideNav and overlay behaviour on click #3666

nxn-4-wdf opened this issue Aug 7, 2019 · 13 comments · Fixed by #8296
Assignees
Labels
component: ui-shell package: react carbon-components-react severity: 3 https://ibm.biz/carbon-severity type: bug 🐛

Comments

@nxn-4-wdf
Copy link

What package(s) are you using?

  • carbon-components
  • carbon-components-react

Detailed description

I use the UI shell with a SideNav, and a HashRouter, similar to what's described in the Carbon React tutorial.

On small or medium viewports, when you click on the HeaderMenuButton, the SideNav is expanded, and the rest of the page content is masked by a gray overlay.

Then there are 2 related issues:

  1. If the user clicks on a SideNavLink, the content loads into the content area, but the SideNav remains expanded, and does not close, as is expected.
  2. If the user wants to close the SideNav (as a result of 1, or simply because he wants to remain on the same page), he must click on the HeaderMenuButton (currently displaying as a cross). Clicking anywhere on the overlay or the menu bar does not close the SideNav as expected.

In both cases, user expects the SideNav to be closed after a click inside or outside of it, because that's the way most apps behave.

Bug seems browser-independent.
Using Node.js 10.16.2 (LTS). Dependencies and versions from package.json:

  "dependencies": {
    "@carbon/grid": "^10.4.0",
    "@carbon/icons-react": "^10.4.1",
    "carbon-components": "^10.4.1",
    "carbon-components-react": "^7.4.1",
    "carbon-icons": "^7.0.7",
    "react": "^16.8.6",
    "react-dom": "^16.8.6",
    "react-router-dom": "^5.0.1",
    "react-scripts": "3.0.1"
  },

Steps to reproduce the issue

Setup project with the UI Shell like in the Carbon React tutorial:

  • use and from react-router-dom
  • App structure (stripped pseudo-code, does not contain the Router):
<HeaderContainer render={ ({ isSideNavExpanded, onClickSideNavExpand }) => (
  <>
    <Header>
      <SkipToContent />
      <HeaderMenuButton onClick={onClickSideNavExpand} isActive={isSideNavExpanded} />
      <HeaderName>Name</HeaderName>
      <SideNav expanded={isSideNavExpanded} isPersistent={false}>
        <SideNavItems>
          <SideNavLink element={Link} to="link1">Link 1</SideNavLink>
          <SideNavLink element={Link} to="link2">Link 2</SideNavLink>
        </SideNavItems>
      </SideNav>
    </Header>
    <p>Content area</p>
  </>
  )}
/>

Additional information

Picture below shows a prototype: clicking on any of the zones pointed to by an arrow should close the side navigation.

image

@nxn-4-wdf
Copy link
Author

React code example reproducing the issue: https://codesandbox.io/s/sidenavissues-jtrzb

Result here: https://jtrzb.csb.app/#/
To reproduce:

  • Reduce viewport width to less than 40rem, so that the HeaderMenuButton is displayed on the top-left corner.
  • Click on the menu button to display the SideNav.
  • Click on the different zones (see picture above in section "Additional information") does not close the SideNav, as expected.

Related issues with keyboard navigation:

  • pressing the Enter-key when a SideNavLink is selected does not close the side navigation
  • pressing the ESC-key does not close the side navigation

@aledavila aledavila added component: ui-shell package: react carbon-components-react priority: high severity: 3 https://ibm.biz/carbon-severity labels Dec 6, 2019
@dryhurst
Copy link

dryhurst commented Dec 27, 2019

@nxn-4-wdf hey man, i ran into this problem too. just change your <Header component to look like this:

<HeaderContainer
        render={({ isSideNavExpanded, onClickSideNavExpand }) => (
          <Fragment>
            <Header
              aria-label="nxn-4-wdf's Totally Tubular Application Worth Millions"
              onClick={
                isSideNavExpanded === true ? onClickSideNavExpand : null
              }>

tadaaaaa

@dryhurst
Copy link

dryhurst commented Dec 27, 2019

@nxn-4-wdf yo. i ran into another problem where the sidenav stays there if you resize the browser so i amended the code to look like this. this fixes both the click out issue you describes as well as a broken ghosted sidenav on resize

      <HeaderContainer
        render={({ isSideNavExpanded, onClickSideNavExpand }) => {
          window.addEventListener(
            'resize',
            () => {
              const viewportWidth =
                window.innerWidth || document.documentElement.clientWidth;
              if (viewportWidth > 1056) {
                if (isSideNavExpanded === true) onClickSideNavExpand();
              }
            },
            false
          );
          return (
            <Fragment>
              <Header
                aria-label="Carbon Tutorial"
                onClick={
                  isSideNavExpanded === true ? onClickSideNavExpand : null
                }>

@jalbertogg
Copy link

jalbertogg commented May 7, 2020

@dryhurst Thanks for your code!
I have used it in my code and solves all the use cases except one:

  • It calls onClickSideNavExpand on clicking on the sidebar link items, but does not hide the sidenav panel. It is weird because I can see how it removes the overlay background and change the header menu button... I think maybe the nav items is preventing it to be closed when they get the focus.
<HeaderContainer
          render={({ isSideNavExpanded, onClickSideNavExpand }) => {
            window.addEventListener(
              'resize',
              () => {
                const viewportWidth =
                  window.innerWidth || document.documentElement.clientWidth;
                if (viewportWidth > 1056) {
                  if (isSideNavExpanded === true) onClickSideNavExpand();
                }
              },
              false
            )

            return (
              <Fragment>
                <Header
                  aria-label="my app"
                  onClick={ isSideNavExpanded === true ? onClickSideNavExpand : null}
                  >
                  <SkipToContent />
                  <HeaderMenuButton
                    aria-label="Open menu"
                    onClick={onClickSideNavExpand}
                    isActive={isSideNavExpanded}
                  />
                  <HeaderName element={NavLink} to="/" prefix="">
                    MyApp
                  </HeaderName>
                  <HeaderNavigation aria-label="Carbon Tutorial">
                    <HeaderMenuItem element={NavLink} to="/about">About</HeaderMenuItem>
                    <HeaderMenuItem element={NavLink} to="/help">Help</HeaderMenuItem>
                    <HeaderMenuItem element={NavLink} to="/contact">Contact</HeaderMenuItem>
                    <HeaderMenuItem element={NavLink} to="/theme">Theme</HeaderMenuItem>
                  </HeaderNavigation>
                  <SideNav
                    aria-label="Side navigation"
                    expanded={isSideNavExpanded}
                    isPersistent={false}>
                    <SideNavItems>
                      <HeaderSideNavItems>
                        <HeaderMenuItem element={NavLink} to="/about">About</HeaderMenuItem>
                        <HeaderMenuItem element={NavLink} to="/help">Help</HeaderMenuItem>
                        <HeaderMenuItem element={NavLink} to="/contact">Contact</HeaderMenuItem>
                        <HeaderMenuItem element={NavLink} to="/theme">Theme</HeaderMenuItem>
                      </HeaderSideNavItems>
                    </SideNavItems>
                  </SideNav>
                </Header>
              </Fragment>
          )}}
        />

Thanks!

@jalbertogg
Copy link

jalbertogg commented May 7, 2020

Hey, I have solved it with a workaround, I have identified that by clicking on the nav items it didn't remove the bx--side-nav--expanded class of the side nav panel.

<SideNav
   aria-label="Side navigation"
   expanded={isSideNavExpanded}
   isPersistent={false}
   className="global_sidenav">
      <SideNavItems>
         <HeaderSideNavItems>
            <HeaderMenuItem element={NavLink} to="/about" onClick={hideSideNav}>About</HeaderMenuItem>
            <HeaderMenuItem element={NavLink} to="/help" onClick={hideSideNav}>Help</HeaderMenuItem>
            <HeaderMenuItem element={NavLink} to="/contact" onClick={hideSideNav}>Contact</HeaderMenuItem>
            <HeaderMenuItem element={NavLink} to="/theme" onClick={hideSideNav}>Theme</HeaderMenuItem>
         </HeaderSideNavItems>
      </SideNavItems>
</SideNav>
const hideSideNav = () => {
    var sidenav = document.getElementsByClassName('global_sidenav');
    sidenav[0].classList.remove('bx--side-nav--expanded');
  };`

@ndtx
Copy link

ndtx commented May 12, 2020

@nxn-4-wdf hey man, i ran into this problem too. just change your <Header component to look like this:

<HeaderContainer
        render={({ isSideNavExpanded, onClickSideNavExpand }) => (
          <Fragment>
            <Header
              aria-label="nxn-4-wdf's Totally Tubular Application Worth Millions"
              onClick={
                isSideNavExpanded === true ? onClickSideNavExpand : null
              }>

tadaaaaa

this will trigger when you try to expand a sidenav menu as well.

let onHeaderClick = (evt, hprops) => {
  if(evt.target.className.includes && evt.target.className.includes('nav__overlay')) {
    hprops.onClickSideNavExpand(evt)
  }
}

<HeaderContainer render={hprops => (
  <Header aria-label="example" onClick={(evt) => {onHeaderClick(evt, hprops)}}>
)} />

@JQC5610
Copy link

JQC5610 commented May 16, 2020

HeaderMenuButton not rendering on Chrome but I can see it when I open the CodeSandBox, any reason for this?

@ndtx
Copy link

ndtx commented May 16, 2020

HeaderMenuButton not rendering on Chrome but I can see it when I open the CodeSandBox, any reason for this?

<HeaderMenuButton
    aria-label="Open menu"
    onClick={onClickSideNavExpand}
    isActive={isSideNavExpanded}
/>

make sure you have this added. also.. this appears only on low width pages. (max ~1050px)

@propovip
Copy link

propovip commented Jun 25, 2020

Is there a way to display the HeaderMenuButton even if the page width is more than 1050px? And when the HeaderMenuButton is expanded the background is not greyed out.

@jmmadsen
Copy link

^would like to second this question. Basically, is there a way to turn responsiveness off through props or something? It seems odd it forces you to use responsiveness.

@emyarod
Copy link
Member

emyarod commented Mar 23, 2021

cc @carbon-design-system/design do we have a spec that covers the expand/collapse behavior for the sidenav? also is the expand/collapse behavior only available at smaller screen sizes?

@thyhmdo thyhmdo self-assigned this Mar 30, 2021
@thyhmdo
Copy link
Member

thyhmdo commented Mar 30, 2021

@here So the current Storybook may have a bug issue with it. But this old issue has the correct specified interaction spec in its and should resolve the current issues with the SideNav in Storybook.

#2335 (comment)

@thyhmdo
Copy link
Member

thyhmdo commented Mar 30, 2021

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: ui-shell package: react carbon-components-react severity: 3 https://ibm.biz/carbon-severity type: bug 🐛
Projects
None yet
Development

Successfully merging a pull request may close this issue.