Add RB.MenuView, a common JavaScript view for menus.

Review Request #10906 — Created Feb. 18, 2020 and submitted

Information

Review Board
release-4.0.x
360ed51...

Reviewers

This provides a standard menu implementation that we can use any time we
need a menu. It supports dynamically adding items and opening/closing
the menu (with and without animation).

All accessibility needs are covered by this class. It handles setting
the appropriate ARIA roles on the menu, menu items, and the controlling
element that opens the menu. It also handles full keyboard navigation,
allowing the menu to be interacted with entirely by keyboard.

Unit tests cover all the rendering options, focus management, ARIA
attributes/states, and keyboard accessibility available in this
component.

Unit tests passed.

Used this with an upcoming change that updates RB.SplitButtonView to
use RB.MenuView.

Description From Last Updated

Col: 65 Missing semicolon.

reviewbotreviewbot

Col: 61 Missing semicolon.

reviewbotreviewbot

Col: 68 Missing semicolon.

reviewbotreviewbot

Col: 67 Missing semicolon.

reviewbotreviewbot

Shouldn't this be number? Probably want to list the enumerations here like they are in the docs for initialize too.

daviddavid

We should put a cross-reference there too. Even better would be if the JS could introspect the element CSS and …

daviddavid

Can you print what the invalid value was?

daviddavid

There's nothing dynamic about this--why not just put it in the HTML inside $()?

daviddavid

Missing "Args:"

daviddavid

Missing "Args:"

daviddavid
Checks run (1 failed, 1 succeeded)
flake8 passed.
JSHint failed.

JSHint

chipx86
chipx86
david
  1. 
      
  2. Show all issues

    Shouldn't this be number? Probably want to list the enumerations here like they are in the docs for initialize too.

  3. Show all issues

    We should put a cross-reference there too.

    Even better would be if the JS could introspect the element CSS and pull out the value.

  4. Show all issues

    Can you print what the invalid value was?

  5. Show all issues

    There's nothing dynamic about this--why not just put it in the HTML inside $()?

  6. Show all issues

    Missing "Args:"

  7. Show all issues

    Missing "Args:"

  8. 
      
chipx86
david
  1. Ship It!
  2. 
      
chipx86
Review request changed
Status:
Completed
Change Summary:
Pushed to release-4.0.x (72d5e57)