Re: WAVE-291: Disabled submenu in toolbar incorrectly opens when clicked

[t.br...@gmail.com]

I have updated the review with a small additional change.

Rationale:
While a menu is open, you can still use the keyboard to move the
selection (I'd say it's a bug, but I don't know how to fix it and
unfortunately don't have time to investigate), which can cause the
SubmenuToolbarWidget to be disabled.
In this case, the second patch set will auto-hide the menu; whereas with
the first one, the menu would stay open, and you'd have to check the
conditions for each action. With the change, you can (almost) safely
assume your selection hasn't changed since the menu was opened.

http://codereview.waveprotocol.org/616001

[t.br...@gmail.com]

Hmm, an undesirable side-effect of this whole CL is that when a menu in
a toolbar itself has a submenu; it closes as soon as you open the
submenu.
Additionally, the auto-hide from patch set 2 won't close a submenu
(because the menu items are not notified when their parent menu is
closed).

I think that was the reason for the additional ToolbarToggleButton; and
the real fix should be somehow copy the ToolbarToggleButton behavior
within SubmenuToolbarWidget, rather than relying on a
ToolbarToggleButton.Listener.

http://codereview.waveprotocol.org/616001

[hear...@google.com]

+Ben, who wrote the toolbar code.

http://codereview.waveprotocol.org/616001/diff/4001/5001
File
src/org/waveprotocol/wave/client/widget/toolbar/SubmenuToolbarWidget.java
(right):

http://codereview.waveprotocol.org/616001/diff/4001/5001#newcode45
Line 45: public final class SubmenuToolbarWidget extends
ToolbarToggleButton
The javadoc for ToolbarToggleButton says not to extend it.

(apparently it was supposed to be final)

http://codereview.waveprotocol.org/616001