Summary

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

Review Request #10906 — Created Feb. 18, 2020 and submitted March 2, 2020, 5:30 p.m.

Information

Owner

chipx86

Repository

Review Board

Branch

release-4.0.x

Bugs

Depends On

Commit

360ed51...

Reviewers

Groups

reviewboard

People

Description

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.

Testing Done

Unit tests passed.

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

Issues

Description	From	Last Updated
Col: 65 Missing semicolon.	reviewbot	Feb. 18, 2020, 5:57 p.m.
Col: 61 Missing semicolon.	reviewbot	Feb. 18, 2020, 5:57 p.m.
Col: 68 Missing semicolon.	reviewbot	Feb. 18, 2020, 5:57 p.m.
Col: 67 Missing semicolon.	reviewbot	Feb. 18, 2020, 5:57 p.m.
Shouldn't this be number? Probably want to list the enumerations here like they are in the docs for initialize too.	david	Feb. 24, 2020, 4:35 p.m.
We should put a cross-reference there too. Even better would be if the JS could introspect the element CSS and …	david	Feb. 24, 2020, 4:41 p.m.
Can you print what the invalid value was?	david	Feb. 24, 2020, 4:36 p.m.
There's nothing dynamic about this--why not just put it in the HTML inside $()?	david	Feb. 24, 2020, 4:37 p.m.
Missing "Args:"	david	Feb. 24, 2020, 4:37 p.m.
Missing "Args:"	david	Feb. 24, 2020, 4:37 p.m.

flake8 passed.

JSHint failed.

JSHint

reviewboard/static/rb/js/ui/views/tests/menuViewTests.es6.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 65
 Missing semicolon.
```
reviewboard/static/rb/js/ui/views/tests/menuViewTests.es6.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 61
 Missing semicolon.
```
reviewboard/static/rb/js/ui/views/tests/menuViewTests.es6.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 68
 Missing semicolon.
```
reviewboard/static/rb/js/ui/views/tests/menuViewTests.es6.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 67
 Missing semicolon.
```

Change Summary:

Fixed some missing semicolons.

Commit:

-	0701e1ea17cfd0febcfeeb4c53b123af55a71d53
+	b79c2757899b772200314c32566bb551610618d1

Diff:

Revision 2 (+958 -3)

Show changes

	reviewboard/staticbundles.py
	reviewboard/static/rb/css/ui/menus.less
	reviewboard/static/rb/js/ui/views/menuView.es6.js
	reviewboard/static/rb/js/ui/views/tests/menuViewTests.es6.js

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:


Mouse movements over menu items now changes focus to that item, meaning that mouse and keyboard no longer fight over visual selection.
Fleshed out documentation.
Made some attributes private.

Commit:

-	b79c2757899b772200314c32566bb551610618d1
+	5283fcfc37c7ed202a204f31bfbce0164025cd5b

Diff:

Revision 3 (+998 -3)

Show changes

	reviewboard/staticbundles.py
	reviewboard/static/rb/css/ui/menus.less
	reviewboard/static/rb/js/ui/views/menuView.es6.js
	reviewboard/static/rb/js/ui/views/tests/menuViewTests.es6.js

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/static/rb/js/ui/views/menuView.es6.js (Diff revision 3)

The issue has been resolved. Show all issues

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

reviewboard/static/rb/js/ui/views/menuView.es6.js (Diff revision 3)

The issue has been resolved. 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.

reviewboard/static/rb/js/ui/views/menuView.es6.js (Diff revision 3)
The issue has been resolved. Show all issues
```
Can you print what the invalid value was?
```
reviewboard/static/rb/js/ui/views/menuView.es6.js (Diff revision 3)
The issue has been resolved. Show all issues
```
There's nothing dynamic about this--why not just put it in the HTML inside $()?
```
reviewboard/static/rb/js/ui/views/menuView.es6.js (Diff revision 3)
The issue has been resolved. Show all issues
```
Missing "Args:"
```
reviewboard/static/rb/js/ui/views/menuView.es6.js (Diff revision 3)
The issue has been resolved. Show all issues
```
Missing "Args:"
```

Change Summary:


Fixed some missing/incorrect bits of documentation.
Fixed some logging to show the incorrect menu type, when provided.
Simplified button menu item construction to provide type="button" during initial construction.

Commit:

-	5283fcfc37c7ed202a204f31bfbce0164025cd5b
+	360ed51334a35a68c4957ba3e4f204e003849bd8

Diff:

Revision 4 (+1017 -3)

Show changes

	reviewboard/staticbundles.py
	reviewboard/static/rb/css/ui/menus.less
	reviewboard/static/rb/js/ui/views/menuView.es6.js
	reviewboard/static/rb/js/ui/views/tests/menuViewTests.es6.js

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-4.0.x (72d5e57)