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

DropdownButtons need an overhaul. #526

Closed
22 of 30 tasks
mtscout6 opened this issue Apr 17, 2015 · 17 comments
Closed
22 of 30 tasks

DropdownButtons need an overhaul. #526

mtscout6 opened this issue Apr 17, 2015 · 17 comments
Assignees

Comments

@mtscout6
Copy link
Member

I know there are a lot of problems with the DropdownButton, which I hope to remedy soon. I know I'll have some time to work on them next week, but I can't guarantee a delivery date. I'm mainly creating this issue to aggregate all the dropdown issues in one place.

Notes as per W3C WAI-ARIA Spec:

  • When presenting the menu, ensure that it is completely visible on screen.
  • Keyboard navigation. Should resolve [fixed] Keyboard traversing on DropdownMenu #588
    • Space or Enter - With focus on the button pressing Space or Enter will toggle the display of the drop-down menu. Focus remains on the button.
    • Down Arrow
      • With focus on the button and no drop-down menu displayed, pressing Down Arrow will open the drop-down menu and move focus into the menu and onto the first menu item.
      • With focus on the button and the drop-down menu open, pressing Down Arrow will move focus into the menu onto the first menu item.
    • Up and Down Arrow
      • With focus on the drop-down menu, the Up and Down Arrow keys move focus within the menu items, "wrapping" at the top and bottom.
    • Escape
    • Tab
      • With focus on the button pressing the Tab key will take the user to the next tab focusable item on the page.
      • With focus on the drop-down menu, pressing the Tab key will take the user to the next tab focusable item on the page. Note that this may be difficult to achieve on a web page.
      • Typing a letter (printable character) key moves focus to the next instance of a visible node whose title begins with that printable letter.
  • The menu button itself has a role of button. It is a button so not needed.
  • The menu button has an aria-haspopup property, set to true.
  • Auto-generate an ID if one is not provided.

Notes per reported issues:

Existing features that should still be supported:

  • pullRight
  • dropup
  • noCaret
  • DropdownButton onClick event handler
  • bsSize
  • bsStyle

Issues I Need to dig into more to understand completely:

twbs/bootstrap#15263 - for reference. (Applies some Aria roles I hadn't considered up till now)

@manveru
Copy link

manveru commented Apr 19, 2015

I'm not sure if there's a bug for this yet, but nesting dropdowns results in really broken menus.

@mtscout6
Copy link
Member Author

Thanks @manveru I will take that into consideration.

@mtscout6
Copy link
Member Author

For those of you following this issue, I have started on this though I don't have anything of note to push yet. I should get a little bit more time this week to work on this issue.

@mtscout6
Copy link
Member Author

mtscout6 commented May 5, 2015

@manveru Support for sub menus was dropped in Bootstrap 3 so I don't think we will be including it here. So, you'll need to have your own custom component to do this. There is a good solution proposed on StackOverflow.

I am thinking about allowing for a CustomDropdownMenu as outlined in #470, which would be a great extensibility point for you to add this feature to your project. I'm still fleshing that out though so hopefully that ends up working out.

@mtscout6
Copy link
Member Author

createChainedFunction cleanup: From @jtenner on gitter

@AlexKVal
Copy link
Member

I've edited the first message:

@MatthewHerbst
Copy link

Can you please clarify if this is taking into account the root cause of #368 ?

I currently have the following React render call for my Nav component:

render: function () {
    var projectMenuItemLinks = this.state.projects.map(function(project, i) {
      return (
        <MenuItemLink
          key={i}
          eventKey={'\'' + (i + 1) + '\''}
          to='validate'
          params={{projectId: project.id}}
        >
          {project.name}
        </MenuItemLink>
      );
    });

    return (
      <Navbar brand='My Project' inverse toggleNavKey={0}>
        <Nav right eventKey={0}> {}
          <NavItemLink eventKey={1} to='home' params={{}}>Home</NavItemLink>
          <DropdownButton eventKey={2} title='Projects'>
            {projectMenuItemLinks}
          </DropdownButton>
          <NavItemLink eventKey={3} to='leaderboard' params={{}}>Leaderboard</NavItemLink>
          <NavItemLink eventKey={4} to='profile' params={{}}>Profile</NavItemLink>
        </Nav>
      </Navbar>
    );
  }

This is using react-bootstrap for everything except the MenuItemLink which is a wrapper around MenuItem created by react-router-boostrap.

When selecting one of the links from the dropdown, I would like the dropdown to close (in addition to the page navigation). Do/will I need to specify an onSelect as was being discussed in #368?

I was also taking a look at https://rackt.github.io/react-router/#Transition, but it's not obvious to me how to integrate it with react-router-bootstrap since I don't have direct access to set/remove the "open" class.

@mtscout6
Copy link
Member Author

@MatthewHerbst yes the changes in the dropdown-revisited branch which is PR #1096 should fix the underlying problem. Though we are investigating an alternative approach that will be easier to work with. So, once #1096 is merged we'll need to update the react-router-bootstrap project to fix the problem you're describing.

@MatthewHerbst
Copy link

@mtscout6 awesome, thanks for the update!

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

No branches or pull requests

6 participants