Remove EXPANDED state from Reader Mode

Remove EXPANDED state from Reader Mode

It does not make sense for reader to have a state between peeking
and maximized. This change adds support for disabling the expanded
state for an OverlayPanel.

BUG=565585
Committed: https://crrev.com/abe355b9079ce4d4a23f8abc36d7efde417c7597
Cr-Commit-Position: refs/heads/master@{#371292}

[mdjones, 2016-01-22 01:20:12]

mdjones@chromium.org changed reviewers:
+ twellington@chromium.org

[mdjones, 2016-01-22 01:20:12]

The amount that you have to swipe to get to the next state in this case might
require some tuning. Think that can be done in another patch though. PTAL

[Theresa, 2016-01-22 19:41:06]

https://codereview.chromium.org/1619633003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanelBase.java
(right):

https://codereview.chromium.org/1619633003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanelBase.java:777:
* @return If the the panel supports an expanded state rather than just PEEKING
or MAXIMIZED.
nit: make EXPANDED all-caps since peeking and maximized are all caps?

https://codereview.chromium.org/1619633003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanelBase.java:1174:
getBarHeightExpanded(),
Should this be special cased to getBarHeightPeeking() if we don't have an
expanded state? I don't actually know what the bar height does between all three
states...

https://codereview.chromium.org/1619633003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/readermode/ReaderModePanel.java
(right):

https://codereview.chromium.org/1619633003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/readermode/ReaderModePanel.java:232:
getReaderModeBarControl().setBarText(R.string.reader_mode_expanded_title);
nit: rename string reader_mode_maximized_title?

https://codereview.chromium.org/1619633003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/readermode/ReaderModePanel.java:240:
// TODO(mdjones): This check will be unnecessary after the expanded state is
removed.
Is this check now unnecessary?

https://codereview.chromium.org/1619633003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/readermode/ReaderModePanel.java:251:
boolean animatingToOpenState = getPanelState() == PanelState.EXPANDED
Remove PanelState.EXPANDED from this line?

[mdjones, 2016-01-22 21:15:44]

https://codereview.chromium.org/1619633003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanelBase.java
(right):

https://codereview.chromium.org/1619633003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanelBase.java:777:
* @return If the the panel supports an expanded state rather than just PEEKING
or MAXIMIZED.
On 2016/01/22 19:41:05, Theresa Wellington wrote:
> nit: make EXPANDED all-caps since peeking and maximized are all caps?

Done.

https://codereview.chromium.org/1619633003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanelBase.java:1174:
getBarHeightExpanded(),
On 2016/01/22 19:41:05, Theresa Wellington wrote:
> Should this be special cased to getBarHeightPeeking() if we don't have an
> expanded state? I don't actually know what the bar height does between all
three
> states...

Done.

https://codereview.chromium.org/1619633003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/readermode/ReaderModePanel.java
(right):

https://codereview.chromium.org/1619633003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/readermode/ReaderModePanel.java:232:
getReaderModeBarControl().setBarText(R.string.reader_mode_expanded_title);
On 2016/01/22 19:41:06, Theresa Wellington wrote:
> nit: rename string reader_mode_maximized_title?

Done.

https://codereview.chromium.org/1619633003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/readermode/ReaderModePanel.java:240:
// TODO(mdjones): This check will be unnecessary after the expanded state is
removed.
On 2016/01/22 19:41:06, Theresa Wellington wrote:
> Is this check now unnecessary?

Done.

https://codereview.chromium.org/1619633003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/readermode/ReaderModePanel.java:251:
boolean animatingToOpenState = getPanelState() == PanelState.EXPANDED
On 2016/01/22 19:41:06, Theresa Wellington wrote:
> Remove PanelState.EXPANDED from this line?

Done.

[Theresa, 2016-01-25 18:51:55]

lgtm

[mdjones, 2016-01-25 18:58:39]

mdjones@chromium.org changed reviewers:
+ dfalcantara@chromium.org

[mdjones, 2016-01-25 18:58:40]

+dfalcantara: android_chrome_strings.grd please

[gone, 2016-01-25 18:59:53]

Strings lgtm.

[mdjones, 2016-01-25 19:07:57]

The CQ bit was checked by mdjones@chromium.org

[commit-bot: I haz the power, 2016-01-25 19:09:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1619633003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1619633003/20001

[commit-bot: I haz the power, 2016-01-25 20:12:17]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-01-25 20:14:04]

Description was changed from

==========
Remove EXPANDED state from Reader Mode

It does not make sense for reader to have a state between peeking
and maximized. This change adds support for disabling the expanded
state for an OverlayPanel.

BUG=565585
==========

to

==========
Remove EXPANDED state from Reader Mode

It does not make sense for reader to have a state between peeking
and maximized. This change adds support for disabling the expanded
state for an OverlayPanel.

BUG=565585

Committed: https://crrev.com/abe355b9079ce4d4a23f8abc36d7efde417c7597
Cr-Commit-Position: refs/heads/master@{#371292}
==========

[commit-bot: I haz the power, 2016-01-25 20:14:05]

Patchset 2 (id:??) landed as
https://crrev.com/abe355b9079ce4d4a23f8abc36d7efde417c7597
Cr-Commit-Position: refs/heads/master@{#371292}