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

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

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

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. Shouldn't this be number? Probably want to list the enumerations here like they are in the docs for initialize too.

  3. 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. Can you print what the invalid value was?

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

  6. Missing "Args:"

  7. Missing "Args:"

  8. 
      
chipx86
david
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-4.0.x (72d5e57)
Loading...