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

Issue 876542 link

Starred by 1 user
Status: Verified
Owner:
mdjones@chromium.org
Closed: Sep 10
Cc:
amaralp@chromium.org
█ ramyan@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

[Duet] Most visited tiles transition on fake search box focused on NTP is not correct

Project Member Reported by huayinz@chromium.org, Aug 21 
Chrome Version: 70.0.3529.0
OS: Android N

What steps will reproduce the problem?
(1) Enable Chrome Duet
(2) Open a new tab page
(3) Click on the search box and observe

What is the expected result?
Tiles should be below toolbar

What happens instead?
Tiles overlaps the toolbar
ntp_toolbar_transition_duet.png
138 KB View Download
ntp_toolbar_transition_existing.png
138 KB View Download

Comment 1 by mdjones@chromium.org, Aug 23 

Hm interesting, it looks like the animation for the icons keeps going after the toolbar has stopped moving. I guess those two animations are actually separate.

Comment 2 by mdjones@chromium.org, Aug 23

Cc: -mdjones@chromium.org
Owner: mdjones@chromium.org
Status: Assigned (was: Available)

Comment 3 by twelling...@chromium.org, Aug 28 

Take a look at NewTabPageLayout#getToolbarTransitionPercentage(). There may be some updates needed since the NTP top padding changed for Duet.

Comment 4 by twelling...@chromium.org, Aug 28 

... or NewTabPageLayout#onUrlFocusAnimationChanged() since this is for a URL focus.

Comment 5 by twelling...@chromium.org, Aug 28

Labels: zine-triaged
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 10 

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

commit 0639f5fefed7417e5a3e56fbcc1854c721493567
Author: Matthew Jones <mdjones@chromium.org>
Date: Mon Sep 10 15:42:18 2018

Fix NTP top margin issues

Instead of adjusting the padding for the whole NTP view, this patch
only changes the margin on the logo (if it exists). This causes the
most visited icons to be in the correct position when the omnibox is
focused (with and without logo) and fixes an accessibility focus
highlight bug.

Bug: 879485,  876542 , 881993
Change-Id: If8df92af096a44a18dea83fdaa49a0776f5296c5
Reviewed-on: https://chromium-review.googlesource.com/1214219
Reviewed-by: Theresa <twellington@chromium.org>
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589925}
[modify] https://crrev.com/0639f5fefed7417e5a3e56fbcc1854c721493567/chrome/android/java/res/values/dimens.xml
[modify] https://crrev.com/0639f5fefed7417e5a3e56fbcc1854c721493567/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageLayout.java

Comment 7 by mdjones@chromium.org, Sep 10

Status: Fixed (was: Assigned)
The patch in #6 is being merged via https://crbug.com/879485
Project Member

Comment 8 by bugdroid1@chromium.org, Sep 11

Labels: merge-merged-3538
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/670e6a020d07fefd7914a08e83111840b420d229

commit 670e6a020d07fefd7914a08e83111840b420d229
Author: Matthew Jones <mdjones@chromium.org>
Date: Tue Sep 11 19:51:37 2018

Fix NTP top margin issues

Instead of adjusting the padding for the whole NTP view, this patch
only changes the margin on the logo (if it exists). This causes the
most visited icons to be in the correct position when the omnibox is
focused (with and without logo) and fixes an accessibility focus
highlight bug.

TBR=mdjones@chromium.org

(cherry picked from commit 0639f5fefed7417e5a3e56fbcc1854c721493567)

Bug: 879485,  876542 , 881993
Change-Id: If8df92af096a44a18dea83fdaa49a0776f5296c5
Reviewed-on: https://chromium-review.googlesource.com/1214219
Reviewed-by: Theresa <twellington@chromium.org>
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#589925}
Reviewed-on: https://chromium-review.googlesource.com/1220472
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#287}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/670e6a020d07fefd7914a08e83111840b420d229/chrome/android/java/res/values/dimens.xml
[modify] https://crrev.com/670e6a020d07fefd7914a08e83111840b420d229/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageLayout.java

Comment 9 by kgna...@chromium.org, Sep 13

Status: Verified (was: Fixed)
Fix looks good as per expected  behavior, 
Issue verified on 70.0.3538.17 and 71.0.3551.0
►

Sign in to add a comment