Remove redundant icons from top toolbar when bottom toolbar is activated |
||
Issue descriptionThe tab switcher and overflow menu are redundant when the bottom toolbar is active. They should be hidden.
,
Jun 26 2018
,
Jul 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8438c4f922b5fbe03fc625c59fb00cec72bf91a6 commit 8438c4f922b5fbe03fc625c59fb00cec72bf91a6 Author: Peter E Conn <peconn@chromium.org> Date: Tue Jul 03 14:04:16 2018 Revert "Don't show top toolbar items when bottom toolbar is enabled" This reverts commit e83d1f12d67bfacbc9a6bde53593302326176817. Reason for revert: https://crbug.com/857437 Essentially, as well as adding if statements to all usages of mMenuButton within the class, you need to add if statements to all other classes calling 'getMenuButton()' (and ToolbarManager.getMenuButton() transitively). Unfortunately this isn't so trivial for all the cases (eg, I'm guessing we still want to show the data saver when this feature is enabled, or the download text bubble?), so I'm reverting instead of attempting and TBRing a fix myself. Sorry! Original change's description: > Don't show top toolbar items when bottom toolbar is enabled > > Remove the top toolbar buttons (home, tab switcher, and menu buttons) > from their parent view and set them to null. Also add a bunch of > null checks since now things can be null. > > Bug: 852117 > Change-Id: I620f35f81388d1f4ac8791a9eb45576566474604 > Reviewed-on: https://chromium-review.googlesource.com/1105569 > Commit-Queue: Pedro Amaral <amaralp@chromium.org> > Reviewed-by: Ted Choc <tedchoc@chromium.org> > Cr-Commit-Position: refs/heads/master@{#570578} TBR=tedchoc@chromium.org,amaralp@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 852117 Change-Id: I2c16533f8dec11c5d11b920693157735e7438e89 Reviewed-on: https://chromium-review.googlesource.com/1124579 Commit-Queue: Peter Conn <peconn@chromium.org> Reviewed-by: Peter Conn <peconn@chromium.org> Cr-Commit-Position: refs/heads/master@{#572193} [modify] https://crrev.com/8438c4f922b5fbe03fc625c59fb00cec72bf91a6/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarLayout.java [modify] https://crrev.com/8438c4f922b5fbe03fc625c59fb00cec72bf91a6/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java [modify] https://crrev.com/8438c4f922b5fbe03fc625c59fb00cec72bf91a6/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarPhone.java
,
Jul 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bf930470154ff70cf27b2455114343b964341597 commit bf930470154ff70cf27b2455114343b964341597 Author: Pedro Amaral <amaralp@chromium.org> Date: Mon Jul 16 19:03:30 2018 Don't show top toolbar items when bottom toolbar is enabled The original CL for this (https://chromium-review.googlesource.com/1105569) was reverted because of a null pointer bug. This CL does the same thing as the reverted CL and fixes the null pointer exception by having ToolbarManager#getMenuButton() return the bottom toolbar menu button when the bottom toolbar is enabled. Bug: 852117 Change-Id: Ic2668a3cac6a552bee4476d1cfb58f8e6c290bfd Reviewed-on: https://chromium-review.googlesource.com/1135776 Reviewed-by: Matthew Jones <mdjones@chromium.org> Reviewed-by: Ted Choc <tedchoc@chromium.org> Commit-Queue: Pedro Amaral <amaralp@chromium.org> Cr-Commit-Position: refs/heads/master@{#575372} [modify] https://crrev.com/bf930470154ff70cf27b2455114343b964341597/chrome/android/java/src/org/chromium/chrome/browser/toolbar/BottomToolbarCoordinator.java [modify] https://crrev.com/bf930470154ff70cf27b2455114343b964341597/chrome/android/java/src/org/chromium/chrome/browser/toolbar/MenuButton.java [modify] https://crrev.com/bf930470154ff70cf27b2455114343b964341597/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarLayout.java [modify] https://crrev.com/bf930470154ff70cf27b2455114343b964341597/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java [modify] https://crrev.com/bf930470154ff70cf27b2455114343b964341597/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarPhone.java
,
Jul 18
Issue 863463 has been merged into this issue. |
||
►
Sign in to add a comment |
||
Comment 1 by bugdroid1@chromium.org
, Jun 26 2018