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

Issue metadata

Status: Fixed
Owner:
Closed: Dec 18
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-12-14
OS: Android
Pri: 1
Type: Feature



Sign in to add a comment
link

Issue 914039: [Duet] Make tab switcher bottom toolbar have three buttons

Reported by amaralp@chromium.org, Dec 11 Project Member

Issue description

We are making the tab switcher bottom toolbar more aesthetic and usable by adding a new close all tabs button and moving the new tab button to the middle. Attached is a screenshot.
 
V.1 Tab Switcher %28VTS%29
43.1 KB Download

Comment 1 by bugdroid1@chromium.org, Dec 13

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/83dc750f4d6228c2f0efb916d88b452d2f9b31c1

commit 83dc750f4d6228c2f0efb916d88b452d2f9b31c1
Author: Pedro Amaral <amaralp@chromium.org>
Date: Thu Dec 13 02:57:54 2018

[Duet] Three button tab switcher bottom toolbar

This CL makes the tab switcher bottom bar have three buttons:
close all tabs, new tab, and menu buttons.

Bug:  914039 
Change-Id: Ibf3555e838d6c63327c197dd16b0a5529b02db3f
Reviewed-on: https://chromium-review.googlesource.com/c/1364069
Commit-Queue: Pedro Amaral <amaralp@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616184}
[add] https://crrev.com/83dc750f4d6228c2f0efb916d88b452d2f9b31c1/chrome/android/java/res/drawable-hdpi/ic_close_all_tabs.png
[add] https://crrev.com/83dc750f4d6228c2f0efb916d88b452d2f9b31c1/chrome/android/java/res/drawable-mdpi/ic_close_all_tabs.png
[add] https://crrev.com/83dc750f4d6228c2f0efb916d88b452d2f9b31c1/chrome/android/java/res/drawable-xhdpi/ic_close_all_tabs.png
[add] https://crrev.com/83dc750f4d6228c2f0efb916d88b452d2f9b31c1/chrome/android/java/res/drawable-xxhdpi/ic_close_all_tabs.png
[add] https://crrev.com/83dc750f4d6228c2f0efb916d88b452d2f9b31c1/chrome/android/java/res/drawable-xxxhdpi/ic_close_all_tabs.png
[modify] https://crrev.com/83dc750f4d6228c2f0efb916d88b452d2f9b31c1/chrome/android/java/res/layout/bottom_toolbar_tab_switcher.xml
[modify] https://crrev.com/83dc750f4d6228c2f0efb916d88b452d2f9b31c1/chrome/android/java/src/org/chromium/chrome/browser/toolbar/IncognitoStateProvider.java
[modify] https://crrev.com/83dc750f4d6228c2f0efb916d88b452d2f9b31c1/chrome/android/java/src/org/chromium/chrome/browser/toolbar/NewTabButton.java
[modify] https://crrev.com/83dc750f4d6228c2f0efb916d88b452d2f9b31c1/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java
[modify] https://crrev.com/83dc750f4d6228c2f0efb916d88b452d2f9b31c1/chrome/android/java/src/org/chromium/chrome/browser/toolbar/bottom/BottomToolbarCoordinator.java
[add] https://crrev.com/83dc750f4d6228c2f0efb916d88b452d2f9b31c1/chrome/android/java/src/org/chromium/chrome/browser/toolbar/bottom/BottomToolbarNewTabButton.java
[add] https://crrev.com/83dc750f4d6228c2f0efb916d88b452d2f9b31c1/chrome/android/java/src/org/chromium/chrome/browser/toolbar/bottom/CloseAllTabsButton.java
[modify] https://crrev.com/83dc750f4d6228c2f0efb916d88b452d2f9b31c1/chrome/android/java/src/org/chromium/chrome/browser/toolbar/bottom/TabSwitcherBottomToolbarCoordinator.java
[add] https://crrev.com/83dc750f4d6228c2f0efb916d88b452d2f9b31c1/chrome/android/java/src/org/chromium/chrome/browser/toolbar/bottom/TabSwitcherThemeColorProvider.java
[modify] https://crrev.com/83dc750f4d6228c2f0efb916d88b452d2f9b31c1/chrome/android/java/src/org/chromium/chrome/browser/toolbar/top/ToolbarPhone.java
[modify] https://crrev.com/83dc750f4d6228c2f0efb916d88b452d2f9b31c1/chrome/android/java/strings/android_chrome_strings.grd
[modify] https://crrev.com/83dc750f4d6228c2f0efb916d88b452d2f9b31c1/chrome/android/java_sources.gni
[modify] https://crrev.com/83dc750f4d6228c2f0efb916d88b452d2f9b31c1/tools/metrics/actions/actions.xml

Comment 2 by amaralp@chromium.org, Dec 13

Labels: Merge-Request-72

Comment 3 by sheriffbot@chromium.org, Dec 13

Project Member
Labels: -Merge-Request-72 Merge-Review-72 Hotlist-Merge-Review
This bug requires manual review: There is .grd file changes and we are only 46 days from stable.
Please contact the milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

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

Comment 4 by gov...@chromium.org, Dec 13

Pls update bug with canary result on Friday morning.
This change seems to be big also includes .grd file change which requires string translation. M72 string freeze was on Nov 9th, branch on Nov 29th.May i please know how critical and safe to merge this change to this late in release cycle?

Comment 5 by gov...@chromium.org, Dec 13

NextAction: 2018-12-14

Comment 6 by amaralp@chromium.org, Dec 13

Cc: lzbylut@chromium.org
This change is will only affect users in the Duet (split toolbar) experiment so it should be safe (can disable experiment if something goes wrong). This change is critical for the 72 experiment.

Comment 7 by gov...@chromium.org, Dec 13

If it is critical for M72 experiment, why merge request so late where strings need translation? (FYI: M72 string freeze was on Nov 9th, branch on Nov 29th)

Comment 8 by amaralp@chromium.org, Dec 13

We had some setbacks that led us to have to merge so late. I thought that since this was behind a flag and only enabled for an experiment it would not be a dangerous change. If the string translation is the main issue then I could use pre-existing strings instead of the new strings for the experiment. IDS_MENU_CLOSE_ALL_TABS/IDS_MENU_CLOSE_INCOGNITO_TABS/IDS_MENU_CLOSE_PRIVATE_TABS could replace the new strings I added.

Comment 9 by gov...@chromium.org, Dec 13

Labels: -Merge-Review-72 Merge-Approved-72
Got it, approving merge to M72 branch 3626 based on comment #6 and #8. Pls merge ASAP so i can submit strings for translation. 

Please land string changes by string freeze in future. Thank you.

Comment 10 by amaralp@chromium.org, Dec 13

Thank you! Do you want me to wait until tomorrow's canary to do the merge? Also do you want me to do the cherry-pick with the pre-existing strings? They are good enough for the experiment and that way we don't have to rush the translation.

Comment 11 by gov...@chromium.org, Dec 13

Ok, pls cherry-pick with the pre-existing strings after canary coverage tomorrow. Thank you.

Comment 12 by monor...@bugs.chromium.org, Dec 14

The NextAction date has arrived: 2018-12-14

Comment 13 by bugdroid1@chromium.org, Dec 15

Project Member
Labels: -merge-approved-72 merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d1f3f6bf7c968fb4dcc9905a2b9ff135109e4135

commit d1f3f6bf7c968fb4dcc9905a2b9ff135109e4135
Author: Pedro Amaral <amaralp@chromium.org>
Date: Sat Dec 15 01:42:38 2018

[Duet] Three button tab switcher bottom toolbar

This CL makes the tab switcher bottom bar have three buttons:
close all tabs, new tab, and menu buttons.

(cherry picked from commit 83dc750f4d6228c2f0efb916d88b452d2f9b31c1)

Bug:  914039 
Change-Id: Ibf3555e838d6c63327c197dd16b0a5529b02db3f
Reviewed-on: https://chromium-review.googlesource.com/c/1364069
Commit-Queue: Pedro Amaral <amaralp@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#616184}
Reviewed-on: https://chromium-review.googlesource.com/c/1378972
Reviewed-by: Pedro Amaral <amaralp@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#382}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[add] https://crrev.com/d1f3f6bf7c968fb4dcc9905a2b9ff135109e4135/chrome/android/java/res/drawable-hdpi/ic_close_all_tabs.png
[add] https://crrev.com/d1f3f6bf7c968fb4dcc9905a2b9ff135109e4135/chrome/android/java/res/drawable-mdpi/ic_close_all_tabs.png
[add] https://crrev.com/d1f3f6bf7c968fb4dcc9905a2b9ff135109e4135/chrome/android/java/res/drawable-xhdpi/ic_close_all_tabs.png
[add] https://crrev.com/d1f3f6bf7c968fb4dcc9905a2b9ff135109e4135/chrome/android/java/res/drawable-xxhdpi/ic_close_all_tabs.png
[add] https://crrev.com/d1f3f6bf7c968fb4dcc9905a2b9ff135109e4135/chrome/android/java/res/drawable-xxxhdpi/ic_close_all_tabs.png
[modify] https://crrev.com/d1f3f6bf7c968fb4dcc9905a2b9ff135109e4135/chrome/android/java/res/layout/bottom_toolbar_tab_switcher.xml
[modify] https://crrev.com/d1f3f6bf7c968fb4dcc9905a2b9ff135109e4135/chrome/android/java/src/org/chromium/chrome/browser/toolbar/IncognitoStateProvider.java
[modify] https://crrev.com/d1f3f6bf7c968fb4dcc9905a2b9ff135109e4135/chrome/android/java/src/org/chromium/chrome/browser/toolbar/NewTabButton.java
[modify] https://crrev.com/d1f3f6bf7c968fb4dcc9905a2b9ff135109e4135/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java
[modify] https://crrev.com/d1f3f6bf7c968fb4dcc9905a2b9ff135109e4135/chrome/android/java/src/org/chromium/chrome/browser/toolbar/bottom/BottomToolbarCoordinator.java
[add] https://crrev.com/d1f3f6bf7c968fb4dcc9905a2b9ff135109e4135/chrome/android/java/src/org/chromium/chrome/browser/toolbar/bottom/BottomToolbarNewTabButton.java
[add] https://crrev.com/d1f3f6bf7c968fb4dcc9905a2b9ff135109e4135/chrome/android/java/src/org/chromium/chrome/browser/toolbar/bottom/CloseAllTabsButton.java
[modify] https://crrev.com/d1f3f6bf7c968fb4dcc9905a2b9ff135109e4135/chrome/android/java/src/org/chromium/chrome/browser/toolbar/bottom/TabSwitcherBottomToolbarCoordinator.java
[add] https://crrev.com/d1f3f6bf7c968fb4dcc9905a2b9ff135109e4135/chrome/android/java/src/org/chromium/chrome/browser/toolbar/bottom/TabSwitcherThemeColorProvider.java
[modify] https://crrev.com/d1f3f6bf7c968fb4dcc9905a2b9ff135109e4135/chrome/android/java/src/org/chromium/chrome/browser/toolbar/top/ToolbarPhone.java
[modify] https://crrev.com/d1f3f6bf7c968fb4dcc9905a2b9ff135109e4135/chrome/android/java_sources.gni
[modify] https://crrev.com/d1f3f6bf7c968fb4dcc9905a2b9ff135109e4135/tools/metrics/actions/actions.xml

Comment 14 by amaralp@chromium.org, Dec 18

Status: Fixed (was: Started)

Comment 15 by cr-audit...@appspot.gserviceaccount.com, Dec 19

Project Member
Labels: Merge-Merged-72-3626
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/d1f3f6bf7c968fb4dcc9905a2b9ff135109e4135

Commit: d1f3f6bf7c968fb4dcc9905a2b9ff135109e4135
Author: amaralp@chromium.org
Commiter: amaralp@chromium.org
Date: 2018-12-15 01:42:38 +0000 UTC

[Duet] Three button tab switcher bottom toolbar

This CL makes the tab switcher bottom bar have three buttons:
close all tabs, new tab, and menu buttons.

(cherry picked from commit 83dc750f4d6228c2f0efb916d88b452d2f9b31c1)

Bug:  914039 
Change-Id: Ibf3555e838d6c63327c197dd16b0a5529b02db3f
Reviewed-on: https://chromium-review.googlesource.com/c/1364069
Commit-Queue: Pedro Amaral <amaralp@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#616184}
Reviewed-on: https://chromium-review.googlesource.com/c/1378972
Reviewed-by: Pedro Amaral <amaralp@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#382}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}

Sign in to add a comment