New issue
Advanced search Search tips

Issue 732508 link

Starred by 1 user
Status: Fixed
Owner:
finnur@chromium.org
Closed: Jun 2017
Cc:
twelling...@chromium.org
peter@chromium.org
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Potentially revise image upload order for new photo picker

Project Member Reported by owe...@chromium.org, Jun 12 2017 
What is the current order that images selected with the new photopicker will be provided to the web content?

Top down vs bottom up vs something else?

Someone on Twitter requested it should be in the order the images were selected [1], I'm not 100% sure that would be the right behavior (I can imagine selecting on top photo at the end after having selected a bunch and being confused about why it was separated from the others at the top), but do want to clarify how it works today.

[1] https://twitter.com/kennethrohde/status/874007228768022528

Comment 1 by finnur@chromium.org, Jun 13 2017

Cc: twelling...@chromium.org
Status: Started (was: Untriaged)
Ah, excellent question! 

The pre-existing SelectionDelegate I'm using keeps track of the selection using a Set, which is converted to an ArrayList when the full list is requested. As such, the order is therefore indeterminate :/ but that's an oversight and I'm happy to whip up a change that implements the order we'd like to see. 

It should be really easy to do most recent first (read: top-down, and bottom-up is very simple also). If we want the order of selection, then we'd probably need to keep track of that in the SelectionDelegate (which means altering that class). 

+Theresa, for comments or insights.

Comment 2 by twelling...@chromium.org, Jun 13 2017 

The other clients of selection delegate don't particularly care about the ordering of selected items so I think that whatever policy we apply to the photo picker could apply to the other clients as well. To return the items in order of selection, we could swap out the HashSet tracking the selected items in SelectionDelegate for a LinkedHashSet.
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 13 2017 

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

commit 3741abddbe2c1c03a473d33e68d92902727c8e44
Author: finnur <finnur@chromium.org>
Date: Tue Jun 13 16:53:17 2017

Photo Picker dialog: Order selected photos the same as on screen.

BUG= 732508 ,  656015 

Review-Url: https://codereview.chromium.org/2935093002
Cr-Commit-Position: refs/heads/master@{#479054}

[modify] https://crrev.com/3741abddbe2c1c03a473d33e68d92902727c8e44/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java

Comment 4 by owe...@chromium.org, Jun 13 2017 

Ordering the same as on screen LGTM! Thanks!

Comment 5 by finnur@chromium.org, Jun 14 2017

Status: Fixed (was: Started)
Thanks, Owen. I interpret your comment in such a way that no further action is required. We can reopen this if that's not the case.
►

Sign in to add a comment