Issue 848962 link

Starred by 3 users

Issue metadata

Status:	Available
Owner:	----
Cc:	omrilio@chromium.org kaznacheev@chromium.org newcomer@chromium.org
Components:	UI>Shell>Launcher UI>Shell>Shelf
EstimatedDays:	----
NextAction:	----
OS:	Chrome
Pri:	3
Type:	Bug

Participants' hotlists:
Launcher-Polish

Sort app shortcuts in context menu

Project Member Reported by warx@chromium.org, Jun 1 2018

Issue description

I had discussion with UX, for app shortcut items ordering in context menu, we should follow the style from Android:

When menu is shown below the anchor point, using "normal order"; when menu is shown above the anchor point, using "reverse order" which is a complete reversal of "normal order"

The "normal order" has these rules:
(1) Static (declared in manifest) shortcuts and then dynamic shortcuts;
(2) Within each shortcut type (static and dynamic), shortcuts are sorted
in order of increasing rank according to getRank().

Project Member

Comment 1 by bugdroid1@chromium.org, Jun 11 2018

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

commit 1e9883219c46fc733efbd0fdaef0db232523f1ce
Author: Qiang Xu <warx@google.com>
Date: Mon Jun 11 16:40:33 2018

arc: sort app shortcut items in context menu

changes:
This CL (paired with Android side CL) gets some metadata from Android
to sort the app shortcut items in context menu based on two rules:
(1) Static (declared in manifest) shortcuts and then dynamic shortcuts;
(2) Within each shortcut type (static and dynamic), shortcuts are sorted
in order of increasing rank according to getRank().

This CL is a first step of crbug.com/848962.

Bug: 848962
Test: manual
Change-Id: I10a4e8e1a7d8cb80c69f5bb81f6e2c6abf564645
Reviewed-on: https://chromium-review.googlesource.com/1086118
Commit-Queue: Qiang Xu <warx@google.com>
Reviewed-by: Luis Hector Chavez <lhchavez@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Yury Khmel <khmel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566030}
[modify] https://crrev.com/1e9883219c46fc733efbd0fdaef0db232523f1ce/chrome/browser/chromeos/BUILD.gn
[add] https://crrev.com/1e9883219c46fc733efbd0fdaef0db232523f1ce/chrome/browser/chromeos/arc/app_shortcuts/arc_app_shortcut_item.cc
[modify] https://crrev.com/1e9883219c46fc733efbd0fdaef0db232523f1ce/chrome/browser/chromeos/arc/app_shortcuts/arc_app_shortcut_item.h
[modify] https://crrev.com/1e9883219c46fc733efbd0fdaef0db232523f1ce/chrome/browser/chromeos/arc/app_shortcuts/arc_app_shortcuts_menu_builder.cc
[modify] https://crrev.com/1e9883219c46fc733efbd0fdaef0db232523f1ce/chrome/browser/chromeos/arc/app_shortcuts/arc_app_shortcuts_request.cc
[modify] https://crrev.com/1e9883219c46fc733efbd0fdaef0db232523f1ce/components/arc/common/app.mojom
[modify] https://crrev.com/1e9883219c46fc733efbd0fdaef0db232523f1ce/components/arc/test/fake_app_instance.cc

Comment 2 by warx@chromium.org, Jun 15 2018

Cc: -newcomer@chromium.org
Owner: newcomer@chromium.org

Assigning this bug to newcomer@, I believe you are more familiar with the left work.

The CL in #1 always have "normal order" sorted.
According to the UX, the remaining work is to reverse app shortcuts when menu is shown above the anchor point.

We sort app shortcuts in chrome/: https://cs.chromium.org/chromium/src/chrome/browser/chromeos/arc/app_shortcuts/arc_app_shortcuts_menu_builder.cc?q=app_shortcuts_menu_b&sq=package:chromium&g=0&l=67
In my brief thinking, it sounds like we need to pass "position information" to chrome/.

Thinking the remaining work is not critical, so p1 -> p3.