Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2936743003:
Don't re-sort Chrome OS wifi network list (Closed)
|
Created:
3 years, 6 months ago by Kevin Cernekee Modified:
3 years, 6 months ago CC:
chromium-reviews, kalyank, sadrul Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionDon't re-sort Chrome OS wifi network list
shill (Service::Compare) already sorts the list based on factors like
network preference, signal strength, security, etc. After the Material
Design changes, Chrome started reordering the list based on simpler
criteria, and this results in unconnected wifi services being ordered
by GUID instead of signal strength.
To improve the UX, let's revert back to the old ordering method.
BUG=732984
TEST=manually build + deploy to samus
Review-Url: https://codereview.chromium.org/2936743003
Cr-Commit-Position: refs/heads/master@{#479716}
Committed: https://chromium.googlesource.com/chromium/src/+/81262df9d8ec534bb53421d890e97754cf5a6718
Patch Set 1 #
Total comments: 2
Messages
Total messages: 18 (11 generated)
The CQ bit was checked by cernekee@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Don't re-sort Chrome OS wifi network list shill (Service::Compare) already sorts the list based on factors like network preference, signal strength, security, etc. After the Material Design changes, Chrome started reordering the list based on simpler criteria, and this results in unconnected wifi services being ordered by GUID instead of signal strength. To improve the UX, let's revert back to the old ordering method. BUG=TBD TEST=manually build + deploy to samus ========== to ========== Don't re-sort Chrome OS wifi network list shill (Service::Compare) already sorts the list based on factors like network preference, signal strength, security, etc. After the Material Design changes, Chrome started reordering the list based on simpler criteria, and this results in unconnected wifi services being ordered by GUID instead of signal strength. To improve the UX, let's revert back to the old ordering method. BUG=TBD TEST=manually build + deploy to samus ==========
cernekee@chromium.org changed reviewers: + stevenjb@chromium.org, tbuckley@chromium.org, varkha@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/2936743003/diff/1/ash/system/network/network_... File ash/system/network/network_list.cc (left): https://codereview.chromium.org/2936743003/diff/1/ash/system/network/network_... ash/system/network/network_list.cc:399: return 3; IIRC, Shill already provides this ordering for unconnected networks, correct?
stevenjb@chromium.org changed reviewers: + tdanderson@chromium.org
+tdandersan@ since varkha@ is OOO, although just until tomorrow so we can wait on his return also.
https://codereview.chromium.org/2936743003/diff/1/ash/system/network/network_... File ash/system/network/network_list.cc (left): https://codereview.chromium.org/2936743003/diff/1/ash/system/network/network_... ash/system/network/network_list.cc:399: return 3; On 2017/06/12 22:26:19, stevenjb wrote: > IIRC, Shill already provides this ordering for unconnected networks, correct? Yes, it sorts by technology before considering strength.
On 2017/06/12 22:30:24, Kevin Cernekee wrote: > https://codereview.chromium.org/2936743003/diff/1/ash/system/network/network_... > File ash/system/network/network_list.cc (left): > > https://codereview.chromium.org/2936743003/diff/1/ash/system/network/network_... > ash/system/network/network_list.cc:399: return 3; > On 2017/06/12 22:26:19, stevenjb wrote: > > IIRC, Shill already provides this ordering for unconnected networks, correct? > > Yes, it sorts by technology before considering strength. This LGTM also. It would be nice to file a bug for this explaining the current vs. desired ordering, that way once the CL lands it will be verified by someone on the test team.
Description was changed from ========== Don't re-sort Chrome OS wifi network list shill (Service::Compare) already sorts the list based on factors like network preference, signal strength, security, etc. After the Material Design changes, Chrome started reordering the list based on simpler criteria, and this results in unconnected wifi services being ordered by GUID instead of signal strength. To improve the UX, let's revert back to the old ordering method. BUG=TBD TEST=manually build + deploy to samus ========== to ========== Don't re-sort Chrome OS wifi network list shill (Service::Compare) already sorts the list based on factors like network preference, signal strength, security, etc. After the Material Design changes, Chrome started reordering the list based on simpler criteria, and this results in unconnected wifi services being ordered by GUID instead of signal strength. To improve the UX, let's revert back to the old ordering method. BUG=732984 TEST=manually build + deploy to samus ==========
On 2017/06/12 22:52:54, tdanderson wrote: > This LGTM also. It would be nice to file a bug for this > explaining the current vs. desired ordering, that way > once the CL lands it will be verified by someone on the > test team. Done
The CQ bit was checked by cernekee@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 1, "attempt_start_ts": 1497539318843330, "parent_rev":
"eeb22bd5bbaa417f1265e1fa596321d74d5e7f1e", "commit_rev":
"81262df9d8ec534bb53421d890e97754cf5a6718"}
Message was sent while issue was closed.
Description was changed from ========== Don't re-sort Chrome OS wifi network list shill (Service::Compare) already sorts the list based on factors like network preference, signal strength, security, etc. After the Material Design changes, Chrome started reordering the list based on simpler criteria, and this results in unconnected wifi services being ordered by GUID instead of signal strength. To improve the UX, let's revert back to the old ordering method. BUG=732984 TEST=manually build + deploy to samus ========== to ========== Don't re-sort Chrome OS wifi network list shill (Service::Compare) already sorts the list based on factors like network preference, signal strength, security, etc. After the Material Design changes, Chrome started reordering the list based on simpler criteria, and this results in unconnected wifi services being ordered by GUID instead of signal strength. To improve the UX, let's revert back to the old ordering method. BUG=732984 TEST=manually build + deploy to samus Review-Url: https://codereview.chromium.org/2936743003 Cr-Commit-Position: refs/heads/master@{#479716} Committed: https://chromium.googlesource.com/chromium/src/+/81262df9d8ec534bb53421d890e9... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/81262df9d8ec534bb53421d890e9... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
