New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 835896 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug
Proj-VR
Proj-XR
Proj-XR-VR



Sign in to add a comment

VR: Don't vertically move omnibox during fade-in

Project Member Reported by cjgrant@chromium.org, Apr 23 2018

Issue description

In lieu of the complete specified omnibox-slide-in-then-keyboard-rotate-in sequence, we should not move the omnibox at all.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5a6a94f9c199e413ed1d4d7e9cc2a8d5844f760e

commit 5a6a94f9c199e413ed1d4d7e9cc2a8d5844f760e
Author: Christopher Grant <cjgrant@chromium.org>
Date: Mon Apr 23 18:59:15 2018

VR: Get rid of vertical animation on the omnibox

The full spec for omnibox appearing is a fade-in and slide-in from
below, followed by the keyboard fading and rotating up into place.
Because the keyboard doesn't currently animate as expected, we have the
omnibox slide behind an already visible keyboard, which is not good.
This change makes the omnibox simply fade in at its target location.

BUG= 835896 
R=bshe

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_vr
Change-Id: Idb68cb9120b144aff0251e39586064a0423fbc1e
Reviewed-on: https://chromium-review.googlesource.com/1024240
Reviewed-by: Ian Vollick <vollick@chromium.org>
Commit-Queue: Christopher Grant <cjgrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552776}
[modify] https://crrev.com/5a6a94f9c199e413ed1d4d7e9cc2a8d5844f760e/chrome/browser/vr/ui_scene_creator.cc

Labels: Merge-Request-67
Cc: cma...@chromium.org
cmasso@, apologies for the direct request here - could we bypass the 24-hour auto-approve cycle for this change?  It's a trivial change, and will be very helpful for a demonstration later this week if we can get it into 67.

Thanks for the consideration!

Comment 4 by cmasso@google.com, Apr 23 2018

Labels: -Merge-Request-67 Merge-Approved-67
Approving the merge as per our offline discussion. Make sure to verify the fix in M67 after merging it.
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 23 2018

Labels: -merge-approved-67 merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c6d175f22797f57e97151260700ddafca7de22fb

commit c6d175f22797f57e97151260700ddafca7de22fb
Author: Christopher Grant <cjgrant@chromium.org>
Date: Mon Apr 23 21:26:39 2018

VR: Get rid of vertical animation on the omnibox

The full spec for omnibox appearing is a fade-in and slide-in from
below, followed by the keyboard fading and rotating up into place.
Because the keyboard doesn't currently animate as expected, we have the
omnibox slide behind an already visible keyboard, which is not good.
This change makes the omnibox simply fade in at its target location.

BUG= 835896 
R=bshe

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_vr
Change-Id: Idb68cb9120b144aff0251e39586064a0423fbc1e
Reviewed-on: https://chromium-review.googlesource.com/1024240
Reviewed-by: Ian Vollick <vollick@chromium.org>
Commit-Queue: Christopher Grant <cjgrant@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#552776}
Reviewed-on: https://chromium-review.googlesource.com/1024958
Reviewed-by: Christopher Grant <cjgrant@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#242}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/c6d175f22797f57e97151260700ddafca7de22fb/chrome/browser/vr/ui_scene_creator.cc

Status: Fixed (was: Started)
Tested on a local M-67 checkout cherry-pick, and re-tested by building the tip of the branch after the merge.  Thanks for expediting this, cmasso@!
Labels: Test-Complete
Verified on build 67.3396.29 beta.  Looks good.

Sign in to add a comment