Move the omnibox natively when showing or hiding.

Move the omnibox natively when showing or hiding.

The omnibox drops slightly when fading.  Previously, the omnibox was moved by
adjusting its CSS margins. This is an inefficient way to handle movement (it
wastes space and doesn't always appear smooth).

BUG=641508
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2622593003
Cr-Commit-Position: refs/heads/master@{#442923}
Committed: https://chromium.googlesource.com/chromium/src/+/48a22a1c2870a87e9e6ee10af358717480f72589

[cjgrant, 2017-01-09 16:38:40]

Description was changed from

==========
Move the omnibox natively when showing or hiding.

The omnibox drops slightly when fading.  Previously, the omnibox was moved by
adjusting its CSS margins. This is an inefficient way to handle movement (it
wastes space and doesn't always appear smooth).

BUG=641508
==========

to

==========
Move the omnibox natively when showing or hiding.

The omnibox drops slightly when fading.  Previously, the omnibox was moved by
adjusting its CSS margins. This is an inefficient way to handle movement (it
wastes space and doesn't always appear smooth).

BUG=641508
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[cjgrant, 2017-01-09 16:45:18]

cjgrant@chromium.org changed reviewers:
+ bshe@chromium.org, mthiesse@chromium.org

[mthiesse, 2017-01-09 16:55:59]

lgtm

https://codereview.chromium.org/2622593003/diff/40001/chrome/browser/resource...
File chrome/browser/resources/vr_shell/vr_shell_ui.js (right):

https://codereview.chromium.org/2622593003/diff/40001/chrome/browser/resource...
chrome/browser/resources/vr_shell/vr_shell_ui.js:430: // Make the box fade away
if it's disappearing.
File a bug to do the opacity fading natively as well to keep things in sync and
improve performance.

[bshe, 2017-01-09 22:40:51]

lgtm

https://codereview.chromium.org/2622593003/diff/40001/chrome/browser/resource...
File chrome/browser/resources/vr_shell/vr_shell_ui.js (right):

https://codereview.chromium.org/2622593003/diff/40001/chrome/browser/resource...
chrome/browser/resources/vr_shell/vr_shell_ui.js:436: let yOffset = this.hidden
? this.fadeYOffset : 0;
perhap only add animation if this.hidden is true instead of add a noop
animation?

[cjgrant, 2017-01-10 14:30:44]

https://codereview.chromium.org/2622593003/diff/40001/chrome/browser/resource...
File chrome/browser/resources/vr_shell/vr_shell_ui.js (right):

https://codereview.chromium.org/2622593003/diff/40001/chrome/browser/resource...
chrome/browser/resources/vr_shell/vr_shell_ui.js:436: let yOffset = this.hidden
? this.fadeYOffset : 0;
On 2017/01/09 22:40:51, bshe wrote:
> perhap only add animation if this.hidden is true instead of add a noop
> animation?

It wouldn't be a noop - it's moving the box from the lowered position back up to
it's normal viewing position.

[bshe, 2017-01-10 15:32:08]

https://codereview.chromium.org/2622593003/diff/40001/chrome/browser/resource...
File chrome/browser/resources/vr_shell/vr_shell_ui.js (right):

https://codereview.chromium.org/2622593003/diff/40001/chrome/browser/resource...
chrome/browser/resources/vr_shell/vr_shell_ui.js:436: let yOffset = this.hidden
? this.fadeYOffset : 0;
On 2017/01/10 14:30:44, cjgrant wrote:
> On 2017/01/09 22:40:51, bshe wrote:
> > perhap only add animation if this.hidden is true instead of add a noop
> > animation?
> 
> It wouldn't be a noop - it's moving the box from the lowered position back up
to
> it's normal viewing position.

Acknowledged. Perhaps add comment for yOffset to clarify that it is also used
for
restoring position.

[cjgrant, 2017-01-10 16:13:06]

https://codereview.chromium.org/2622593003/diff/40001/chrome/browser/resource...
File chrome/browser/resources/vr_shell/vr_shell_ui.js (right):

https://codereview.chromium.org/2622593003/diff/40001/chrome/browser/resource...
chrome/browser/resources/vr_shell/vr_shell_ui.js:430: // Make the box fade away
if it's disappearing.
On 2017/01/09 16:55:59, mthiesse wrote:
> File a bug to do the opacity fading natively as well to keep things in sync
and
> improve performance.

Done. crbug/679761

https://codereview.chromium.org/2622593003/diff/40001/chrome/browser/resource...
chrome/browser/resources/vr_shell/vr_shell_ui.js:436: let yOffset = this.hidden
? this.fadeYOffset : 0;
On 2017/01/10 15:32:08, bshe wrote:
> On 2017/01/10 14:30:44, cjgrant wrote:
> > On 2017/01/09 22:40:51, bshe wrote:
> > > perhap only add animation if this.hidden is true instead of add a noop
> > > animation?
> > 
> > It wouldn't be a noop - it's moving the box from the lowered position back
up
> to
> > it's normal viewing position.
> 
> Acknowledged. Perhaps add comment for yOffset to clarify that it is also used
> for
> restoring position.

Done.

[cjgrant, 2017-01-11 15:46:37]

The CQ bit was checked by cjgrant@chromium.org

[cjgrant, 2017-01-11 15:46:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
mthiesse@chromium.org, bshe@chromium.org
Link to the patchset: https://codereview.chromium.org/2622593003/#ps60001
(title: "Add comment; rebase.")

[commit-bot: I haz the power, 2017-01-11 15:46:56]

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, 2017-01-11 16:43:43]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1484149597518830,
"parent_rev": "5768dc4674019fdd12727a6a41ef25aa394381c7", "commit_rev":
"48a22a1c2870a87e9e6ee10af358717480f72589"}

[commit-bot: I haz the power, 2017-01-11 16:44:15]

Description was changed from

==========
Move the omnibox natively when showing or hiding.

The omnibox drops slightly when fading.  Previously, the omnibox was moved by
adjusting its CSS margins. This is an inefficient way to handle movement (it
wastes space and doesn't always appear smooth).

BUG=641508
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Move the omnibox natively when showing or hiding.

The omnibox drops slightly when fading.  Previously, the omnibox was moved by
adjusting its CSS margins. This is an inefficient way to handle movement (it
wastes space and doesn't always appear smooth).

BUG=641508
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2622593003
Cr-Commit-Position: refs/heads/master@{#442923}
Committed:
https://chromium.googlesource.com/chromium/src/+/48a22a1c2870a87e9e6ee10af358...
==========

[commit-bot: I haz the power, 2017-01-11 16:44:16]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/48a22a1c2870a87e9e6ee10af358...