Use modes to switch cvox panel between collapsed, menus, focused, and tutorial

Use modes to switch cvox panel between collapsed, menus, focused, and tutorial

The ChromeVox tutorial didn't close itself properly, it didn't restore the
state of the HTML properly. To enforce that the panel UI is always in one
mode, make the mode explicit with an enum and have a single function to
transition between modes.

Fix a few related issues and ensure that once you're in the tutorial you can
exit easily with the close button or by pressing Escape.

BUG=618090
TEST=Manually test opening menus, incremental search, and tutorial, and closing each by selecting, pressing close button, or pressing escape
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/477fa87081dc2a5161594ff03ff2337898b53d15
Cr-Commit-Position: refs/heads/master@{#429224}

[dmazzoni, 2016-10-27 21:08:13]

Description was changed from

==========
Use modes to switch cvox panel between collapsed, menus, focused, and tutorial

The ChromeVox tutorial didn't close itself properly, it didn't restore the
state of the HTML properly. To enforce that the panel UI is always in one
mode, make the mode explicit with an enum and have a single function to
transition between modes.

Fix a few related issues and ensure that once you're in the tutorial you can
exit easily with the close button or by pressing Escape.

BUG=618090
TEST=Manually test opening menus, incremental search, and tutorial, and closing
each by selecting, pressing close button, or pressing escape
==========

to

==========
Use modes to switch cvox panel between collapsed, menus, focused, and tutorial

The ChromeVox tutorial didn't close itself properly, it didn't restore the
state of the HTML properly. To enforce that the panel UI is always in one
mode, make the mode explicit with an enum and have a single function to
transition between modes.

Fix a few related issues and ensure that once you're in the tutorial you can
exit easily with the close button or by pressing Escape.

BUG=618090
TEST=Manually test opening menus, incremental search, and tutorial, and closing
each by selecting, pressing close button, or pressing escape
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[dmazzoni, 2016-10-27 21:08:31]

The CQ bit was checked by dmazzoni@chromium.org to run a CQ dry run

[dmazzoni, 2016-10-27 21:08:32]

dmazzoni@chromium.org changed reviewers:
+ dtseng@chromium.org

[commit-bot: I haz the power, 2016-10-27 21:09:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-27 22:41:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-27 22:41:27]

Dry run: This issue passed the CQ dry run.

[David Tseng, 2016-10-28 21:24:36]

https://codereview.chromium.org/2454213002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/chromevox/cvox2/background/panel.js
(right):

https://codereview.chromium.org/2454213002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/panel.js:34:
FULLSCREEN_TUTORIAL: 'tutorial'
Is there a non full screen tutorial (and menu)?

https://codereview.chromium.org/2454213002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/panel.js:639:
Panel.setMode(Panel.Mode.COLLAPSED);
Not equivalent to the removed code.

[dmazzoni, 2016-10-28 21:29:13]

https://codereview.chromium.org/2454213002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/chromevox/cvox2/background/panel.js
(right):

https://codereview.chromium.org/2454213002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/panel.js:34:
FULLSCREEN_TUTORIAL: 'tutorial'
On 2016/10/28 21:24:35, David Tseng wrote:
> Is there a non full screen tutorial (and menu)?

Nope, the idea was just to make it clear what the purpose of the mode was. I
could do that with a comment instead.

I'd like to add options and keyboard explorer as fullscreen modes too.

[dmazzoni, 2016-10-28 21:31:29]

https://codereview.chromium.org/2454213002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/chromevox/cvox2/background/panel.js
(right):

https://codereview.chromium.org/2454213002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/panel.js:639:
Panel.setMode(Panel.Mode.COLLAPSED);
On 2016/10/28 21:24:35, David Tseng wrote:
> Not equivalent to the removed code.

What does setMode do that it shouldn't?

[David Tseng, 2016-10-28 23:02:13]

lgtm

https://codereview.chromium.org/2454213002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/chromevox/cvox2/background/panel.js
(right):

https://codereview.chromium.org/2454213002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/panel.js:34:
FULLSCREEN_TUTORIAL: 'tutorial'
On 2016/10/28 21:29:13, dmazzoni wrote:
> On 2016/10/28 21:24:35, David Tseng wrote:
> > Is there a non full screen tutorial (and menu)?
> 
> Nope, the idea was just to make it clear what the purpose of the mode was. I
> could do that with a comment instead.
> 
> I'd like to add options and keyboard explorer as fullscreen modes too.
> 

Not really an issue besides the enum values being different.

https://codereview.chromium.org/2454213002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/panel.js:259:
this.mode_ = mode;
This is also perhaps a good opportunity to ensure we don't move from certain
modes to others (e.g. tutorial -> menu, focus -> tutorial, or whatever you think
is invalid here).

https://codereview.chromium.org/2454213002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/panel.js:639:
Panel.setMode(Panel.Mode.COLLAPSED);
On 2016/10/28 21:31:29, dmazzoni wrote:
> On 2016/10/28 21:24:35, David Tseng wrote:
> > Not equivalent to the removed code.
> 
> What does setMode do that it shouldn't?

Previously, we only collapsed if we were focused. Just making sure you meant to
always collapse here.

[dmazzoni, 2016-11-01 22:31:22]

https://codereview.chromium.org/2454213002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/chromevox/cvox2/background/panel.js
(right):

https://codereview.chromium.org/2454213002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/panel.js:259:
this.mode_ = mode;
On 2016/10/28 23:02:13, David Tseng wrote:
> This is also perhaps a good opportunity to ensure we don't move from certain
> modes to others (e.g. tutorial -> menu, focus -> tutorial, or whatever you
think
> is invalid here).

I think I need to figure out how to support the menu
while the tutorial is open, as a follow-up - or else
disable the menu.

In the meantime, I tested those permutations and they
all work fine or at least they leave you in a consistent
state now, which is much better than before so I want
to land this first and keep improving it.

[dmazzoni, 2016-11-01 22:31:27]

The CQ bit was checked by dmazzoni@chromium.org

[commit-bot: I haz the power, 2016-11-01 22:32:57]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-01 23:02:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-01 23:02:42]

Try jobs failed on following builders:
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dmazzoni, 2016-11-02 06:36:33]

The CQ bit was checked by dmazzoni@chromium.org

[commit-bot: I haz the power, 2016-11-02 06:36:45]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-02 06:58:37]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-11-02 07:00:05]

Description was changed from

==========
Use modes to switch cvox panel between collapsed, menus, focused, and tutorial

The ChromeVox tutorial didn't close itself properly, it didn't restore the
state of the HTML properly. To enforce that the panel UI is always in one
mode, make the mode explicit with an enum and have a single function to
transition between modes.

Fix a few related issues and ensure that once you're in the tutorial you can
exit easily with the close button or by pressing Escape.

BUG=618090
TEST=Manually test opening menus, incremental search, and tutorial, and closing
each by selecting, pressing close button, or pressing escape
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Use modes to switch cvox panel between collapsed, menus, focused, and tutorial

The ChromeVox tutorial didn't close itself properly, it didn't restore the
state of the HTML properly. To enforce that the panel UI is always in one
mode, make the mode explicit with an enum and have a single function to
transition between modes.

Fix a few related issues and ensure that once you're in the tutorial you can
exit easily with the close button or by pressing Escape.

BUG=618090
TEST=Manually test opening menus, incremental search, and tutorial, and closing
each by selecting, pressing close button, or pressing escape
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/477fa87081dc2a5161594ff03ff2337898b53d15
Cr-Commit-Position: refs/heads/master@{#429224}
==========

[commit-bot: I haz the power, 2016-11-02 07:00:06]

Patchset 1 (id:??) landed as
https://crrev.com/477fa87081dc2a5161594ff03ff2337898b53d15
Cr-Commit-Position: refs/heads/master@{#429224}