New issue
Advanced search Search tips

Issue 864701 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 17
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 805612



Sign in to add a comment

Get rid of old shelf UI code once new UI has been default for a while

Project Member Reported by manucornet@chromium.org, Jul 17

Issue description

There's a lot of code that can be cleaned up once the new UI is default.
 
Components: UI>Shell>Shelf
Status: Assigned (was: Untriaged)
This bug has an owner, thus, it's been triaged. Changing status to "assigned".
Labels: M-70
Labels: -M-70 M-71
This should be M-71 as the new shelf UI becomes default in M-70 and I think it's only safe to delete the old code after a while.
Status: Started (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 21

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

commit eaa9ecac07cfaab0eec81072334043830e886256
Author: Manu Cornet <manucornet@chromium.org>
Date: Fri Sep 21 23:58:03 2018

CrOS Shelf: Clean up old UI code

Bug:  864701 
Change-Id: I32134d30e1592926bd812f51083ad959649d056d
Reviewed-on: https://chromium-review.googlesource.com/1238074
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Manu Cornet <manucornet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593382}
[modify] https://crrev.com/eaa9ecac07cfaab0eec81072334043830e886256/ash/BUILD.gn
[modify] https://crrev.com/eaa9ecac07cfaab0eec81072334043830e886256/ash/shelf/overflow_bubble.cc
[modify] https://crrev.com/eaa9ecac07cfaab0eec81072334043830e886256/ash/shelf/overflow_bubble_view.cc
[modify] https://crrev.com/eaa9ecac07cfaab0eec81072334043830e886256/ash/shelf/overflow_button.cc
[modify] https://crrev.com/eaa9ecac07cfaab0eec81072334043830e886256/ash/shelf/overflow_button.h
[delete] https://crrev.com/734e7be98acfdf95b133993a1e685cde24ff7a06/ash/shelf/overflow_button_test_api.cc
[delete] https://crrev.com/734e7be98acfdf95b133993a1e685cde24ff7a06/ash/shelf/overflow_button_test_api.h
[modify] https://crrev.com/eaa9ecac07cfaab0eec81072334043830e886256/ash/shelf/shelf_background_animator.cc
[modify] https://crrev.com/eaa9ecac07cfaab0eec81072334043830e886256/ash/shelf/shelf_button.cc
[modify] https://crrev.com/eaa9ecac07cfaab0eec81072334043830e886256/ash/shelf/shelf_constants.h
[modify] https://crrev.com/eaa9ecac07cfaab0eec81072334043830e886256/ash/shelf/shelf_layout_manager.cc
[modify] https://crrev.com/eaa9ecac07cfaab0eec81072334043830e886256/ash/shelf/shelf_view.cc
[modify] https://crrev.com/eaa9ecac07cfaab0eec81072334043830e886256/ash/shelf/shelf_view_unittest.cc
[modify] https://crrev.com/eaa9ecac07cfaab0eec81072334043830e886256/ash/shelf/shelf_widget.cc
[modify] https://crrev.com/eaa9ecac07cfaab0eec81072334043830e886256/ash/system/accessibility/dictation_button_tray.cc
[modify] https://crrev.com/eaa9ecac07cfaab0eec81072334043830e886256/ash/system/accessibility/select_to_speak_tray.cc
[modify] https://crrev.com/eaa9ecac07cfaab0eec81072334043830e886256/ash/system/palette/palette_tool_manager.cc
[modify] https://crrev.com/eaa9ecac07cfaab0eec81072334043830e886256/ash/system/status_area_widget_delegate.cc
[modify] https://crrev.com/eaa9ecac07cfaab0eec81072334043830e886256/ash/system/tray/tray_background_view.cc
[modify] https://crrev.com/eaa9ecac07cfaab0eec81072334043830e886256/ash/system/tray/tray_constants.h
[modify] https://crrev.com/eaa9ecac07cfaab0eec81072334043830e886256/ash/system/virtual_keyboard/virtual_keyboard_tray.cc
[modify] https://crrev.com/eaa9ecac07cfaab0eec81072334043830e886256/chrome/browser/about_flags.cc
[modify] https://crrev.com/eaa9ecac07cfaab0eec81072334043830e886256/chromeos/chromeos_switches.cc
[modify] https://crrev.com/eaa9ecac07cfaab0eec81072334043830e886256/chromeos/chromeos_switches.h

Could we please keep the old shelf as a flag, or have an option to align the icons to the left?
Hi Daniel,

I understand that changes can be disruptive to some people's workflow and that they will need a little time to adapt, but maintaining several different configurations (especially for something as prominent as the shelf) is not cheap in terms of resources. It multiplies everything by two: time needed to test all the different use cases, asking developers to make sure their new features work in both modes, etc. The designers have spent a lot of time coming up with a really nice looking new UI and I hope you will like it after the first small speed bump of getting used to the new look :-)
Labels: m-72
Bulk moving all M-71 <P-1's to M-72.
Labels: -M-71 -m-71
Labels: -M-72
Cleanup, not tied to any particular milestone.
Status: Fixed (was: Started)
And actually after the CL mentioned in #9 most of the old UI code is now gone. Closing.
Project Member

Comment 14 by bugdroid1@chromium.org, Oct 23

Labels: merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3d45c00d9ff12af85a673b1ba5c8381d258cfea1

commit 3d45c00d9ff12af85a673b1ba5c8381d258cfea1
Author: Manu Cornet <manucornet@chromium.org>
Date: Tue Oct 23 21:02:49 2018

CrOS shelf: some more 'old UI' cleanup

Change-Id: Ie294e13bd2b4eaf14343e328b0d5fb2b22a30251
Bug:  864701 
Reviewed-on: https://chromium-review.googlesource.com/c/1278616
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Manu Cornet <manucornet@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#599642}(cherry picked from commit c334b9d129837feaf35522148893176065fdaf8a)
Reviewed-on: https://chromium-review.googlesource.com/c/1296931
Reviewed-by: Manu Cornet <manucornet@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#273}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/3d45c00d9ff12af85a673b1ba5c8381d258cfea1/ash/shelf/app_list_button.cc
[modify] https://crrev.com/3d45c00d9ff12af85a673b1ba5c8381d258cfea1/ash/shelf/back_button.cc
[modify] https://crrev.com/3d45c00d9ff12af85a673b1ba5c8381d258cfea1/ash/shelf/shelf_background_animator.cc
[modify] https://crrev.com/3d45c00d9ff12af85a673b1ba5c8381d258cfea1/ash/shelf/shelf_background_animator_unittest.cc
[modify] https://crrev.com/3d45c00d9ff12af85a673b1ba5c8381d258cfea1/ash/shelf/shelf_constants.h
[modify] https://crrev.com/3d45c00d9ff12af85a673b1ba5c8381d258cfea1/ash/shelf/shelf_view.cc
[modify] https://crrev.com/3d45c00d9ff12af85a673b1ba5c8381d258cfea1/ash/shelf/shelf_view.h

Labels: CommitLog-Audit-Violation Merge-Without-Approval M-71
Here's a summary of the rules that were executed: 
 - OnlyMergeApprovedChange: Rule Failed -- Revision 3d45c00d9ff12af85a673b1ba5c8381d258cfea1 was merged to refs/branch-heads/3578 branch with no merge approval from a TPM! 
Please explain why this change was merged to the branch!
 - AcknowledgeMerge: Notification Required -- 
Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/3d45c00d9ff12af85a673b1ba5c8381d258cfea1

Commit: 3d45c00d9ff12af85a673b1ba5c8381d258cfea1
Author: manucornet@chromium.org
Commiter: manucornet@chromium.org
Date: 2018-10-23 21:02:49 +0000 UTC

CrOS shelf: some more 'old UI' cleanup

Change-Id: Ie294e13bd2b4eaf14343e328b0d5fb2b22a30251
Bug:  864701 
Reviewed-on: https://chromium-review.googlesource.com/c/1278616
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Manu Cornet <manucornet@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#599642}(cherry picked from commit c334b9d129837feaf35522148893176065fdaf8a)
Reviewed-on: https://chromium-review.googlesource.com/c/1296931
Reviewed-by: Manu Cornet <manucornet@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#273}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}

Sign in to add a comment