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

Issue 829081 link

Starred by 3 users

Issue metadata

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



Sign in to add a comment

VR: replace shadow for 2D modal dialogs

Project Member Reported by asimjour@chromium.org, Apr 4 2018

Issue description

2D modal dialogs have shadow in their View which looks perfect in 2D but doesn't look like the proper shadow when browsing in VR.

The shadow must be removed from the View, and VR should add it's own shadow.
 

Comment 1 by tiborg@chromium.org, Apr 18 2018

Cc: asimjour@chromium.org
Labels: -Type-Bug -Pri-3 -M-68 M-67 Hotlist-VRB-MVP Pri-1 Type-Task
Owner: tiborg@chromium.org

Comment 2 by tiborg@chromium.org, Apr 18 2018

Components: UI>Browser>VR

Comment 3 by tiborg@chromium.org, Apr 18 2018

Status: Started (was: Assigned)

Comment 4 by tiborg@chromium.org, Apr 18 2018

Summary: VR: replace shadow for 2D modal dialogs (was: VR: remove shadow from 2D modal dialogs)
Cc: dbbrooks@chromium.org
 Issue 834435  has been merged into this issue.
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 19 2018

Project Member

Comment 7 by bugdroid1@chromium.org, Apr 19 2018

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

commit 27e1155d7dedb7bf65afbdc2915c6ff1808d4da4
Author: Tibor Goldschwendt <tiborg@chromium.org>
Date: Thu Apr 19 16:33:26 2018

[vr] Add shadow to Android dialogs

Bug:  829081 
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: Ie5c9266bd1f1e239ae9e85ce84d7d342d38129a6
Reviewed-on: https://chromium-review.googlesource.com/1019397
Commit-Queue: Tibor Goldschwendt <tiborg@chromium.org>
Reviewed-by: Amirhossein Simjour <asimjour@chromium.org>
Reviewed-by: Ian Vollick <vollick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552044}
[modify] https://crrev.com/27e1155d7dedb7bf65afbdc2915c6ff1808d4da4/chrome/browser/vr/ui_scene_constants.h
[modify] https://crrev.com/27e1155d7dedb7bf65afbdc2915c6ff1808d4da4/chrome/browser/vr/ui_scene_creator.cc

Comment 8 by tiborg@chromium.org, Apr 19 2018

Labels: Merge-Request-67

Comment 9 by tiborg@chromium.org, Apr 19 2018

Labels: -Merge-Request-67
There is another bug that needs to be fixed.
Project Member

Comment 10 by bugdroid1@chromium.org, Apr 20 2018

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

commit 4b90edc2600e68c9c08bdaf8a3d94f23796e7375
Author: Tibor Goldschwendt <tiborg@chromium.org>
Date: Fri Apr 20 15:24:56 2018

[vr] Fade in shadow with dialog

Prior to this change, the shadow popped in while the hosted UI faded in
causing a weird flicker.

Bug:  829081 
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: I91faf20624219dd88637b27d1322389aafe2aa3d
Reviewed-on: https://chromium-review.googlesource.com/1020360
Reviewed-by: Ian Vollick <vollick@chromium.org>
Commit-Queue: Tibor Goldschwendt <tiborg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552339}
[modify] https://crrev.com/4b90edc2600e68c9c08bdaf8a3d94f23796e7375/chrome/browser/vr/ui_scene_creator.cc

Labels: Merge-Request-67
Project Member

Comment 12 by sheriffbot@chromium.org, Apr 21 2018

Labels: -Merge-Request-67 Merge-Approved-67 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M67. Please go ahead and merge the CL to branch 3396 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 13 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/+/0a84f22e8a73be953a690bf63791f7b74866a9ee

commit 0a84f22e8a73be953a690bf63791f7b74866a9ee
Author: Tibor Goldschwendt <tiborg@chromium.org>
Date: Mon Apr 23 15:12:54 2018

Make shadow/background a property of the modal dialog container

This change is necessary to apply different shadows/backgrounds for
different presenters, e.g. VR.

Bug:  829081 

Change-Id: Ied3b00ab6060ae2c3d2b6acbc1f4612969f36392
Reviewed-on: https://chromium-review.googlesource.com/1017383
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Becky Zhou <huayinz@chromium.org>
Commit-Queue: Tibor Goldschwendt <tiborg@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#552009}(cherry picked from commit 4162d6d214297ae1b00eef49a3321ca0ffc15822)
Reviewed-on: https://chromium-review.googlesource.com/1024092
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#209}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/0a84f22e8a73be953a690bf63791f7b74866a9ee/chrome/android/java/res/layout/modal_dialog_container.xml
[modify] https://crrev.com/0a84f22e8a73be953a690bf63791f7b74866a9ee/chrome/android/java/res/layout/modal_dialog_view.xml
[modify] https://crrev.com/0a84f22e8a73be953a690bf63791f7b74866a9ee/chrome/android/java/src/org/chromium/chrome/browser/modaldialog/AppModalPresenter.java
[modify] https://crrev.com/0a84f22e8a73be953a690bf63791f7b74866a9ee/chrome/android/java/src/org/chromium/chrome/browser/modaldialog/TabModalPresenter.java
[modify] https://crrev.com/0a84f22e8a73be953a690bf63791f7b74866a9ee/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrDialog.java

Project Member

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

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

commit f64c454142d2314d0e0e804853c704727cc07449
Author: Tibor Goldschwendt <tiborg@chromium.org>
Date: Mon Apr 23 15:13:34 2018

[vr] Add shadow to Android dialogs

Bug:  829081 
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: Ie5c9266bd1f1e239ae9e85ce84d7d342d38129a6
Reviewed-on: https://chromium-review.googlesource.com/1019397
Commit-Queue: Tibor Goldschwendt <tiborg@chromium.org>
Reviewed-by: Amirhossein Simjour <asimjour@chromium.org>
Reviewed-by: Ian Vollick <vollick@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#552044}(cherry picked from commit 27e1155d7dedb7bf65afbdc2915c6ff1808d4da4)
Reviewed-on: https://chromium-review.googlesource.com/1023191
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#210}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/f64c454142d2314d0e0e804853c704727cc07449/chrome/browser/vr/ui_scene_constants.h
[modify] https://crrev.com/f64c454142d2314d0e0e804853c704727cc07449/chrome/browser/vr/ui_scene_creator.cc

Project Member

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

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

commit bba06b3748a9b635b82ac6062b72cd35c13f1ff2
Author: Tibor Goldschwendt <tiborg@chromium.org>
Date: Mon Apr 23 15:31:03 2018

[vr] Fade in shadow with dialog

Prior to this change, the shadow popped in while the hosted UI faded in
causing a weird flicker.

TBR=tiborg@chromium.org

(cherry picked from commit 4b90edc2600e68c9c08bdaf8a3d94f23796e7375)

Bug:  829081 
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: I91faf20624219dd88637b27d1322389aafe2aa3d
Reviewed-on: https://chromium-review.googlesource.com/1020360
Reviewed-by: Ian Vollick <vollick@chromium.org>
Commit-Queue: Tibor Goldschwendt <tiborg@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#552339}
Reviewed-on: https://chromium-review.googlesource.com/1023383
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#213}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/bba06b3748a9b635b82ac6062b72cd35c13f1ff2/chrome/browser/vr/ui_scene_creator.cc

Status: Fixed (was: Started)
Labels: Test-Complete

Sign in to add a comment