Views: Reverse Tab order of Download Shelf Items

Reverse the focus order of the download items on the download shelf to aid accessibility.

BUG=145845
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=272667

[Harry McCleave, 2014-05-13 04:02:03]

ptal

[Peter Kasting, 2014-05-13 18:18:10]

Isn't the desired focus order here matching the visible order (i.e. left to
right)?  It seems like instead of having the children go in the opposite of
visual order, and then having to reverse the focus search, we should just ensure
newer downloads are added as earlier children than older ones.  View has a
method to add a child view at a particular index.

[Harry McCleave, 2014-05-13 18:20:21]

On 2014/05/13 18:18:10, Peter Kasting wrote:
> Isn't the desired focus order here matching the visible order (i.e. left to
> right)?  It seems like instead of having the children go in the opposite of
> visual order, and then having to reverse the focus search, we should just
ensure
> newer downloads are added as earlier children than older ones.  View has a
> method to add a child view at a particular index.

Yes this is the case, the old focus would start on the right most download and
tab right to left, this starts on the left most and tabs to the right (for
forward tab), I was under the impression we did not wan't to reverse the order
the downloads are added as we want the most recent to be left most.

[Harry McCleave, 2014-05-13 18:24:34]

On 2014/05/13 18:20:21, Harry McCleave wrote:
> On 2014/05/13 18:18:10, Peter Kasting wrote:
> > Isn't the desired focus order here matching the visible order (i.e. left to
> > right)?  It seems like instead of having the children go in the opposite of
> > visual order, and then having to reverse the focus search, we should just
> ensure
> > newer downloads are added as earlier children than older ones.  View has a
> > method to add a child view at a particular index.
> 
> Yes this is the case, the old focus would start on the right most download and
> tab right to left, this starts on the left most and tabs to the right (for
> forward tab), I was under the impression we did not wan't to reverse the order
> the downloads are added as we want the most recent to be left most.

Belay that comment I misread what you were saying, I will update patch with
suggested insertion.

[Harry McCleave, 2014-05-22 02:14:57]

On 2014/05/13 18:24:34, Harry McCleave wrote:
> On 2014/05/13 18:20:21, Harry McCleave wrote:
> > On 2014/05/13 18:18:10, Peter Kasting wrote:
> > > Isn't the desired focus order here matching the visible order (i.e. left
to
> > > right)?  It seems like instead of having the children go in the opposite
of
> > > visual order, and then having to reverse the focus search, we should just
> > ensure
> > > newer downloads are added as earlier children than older ones.  View has a
> > > method to add a child view at a particular index.
> > 
> > Yes this is the case, the old focus would start on the right most download
and
> > tab right to left, this starts on the left most and tabs to the right (for
> > forward tab), I was under the impression we did not wan't to reverse the
order
> > the downloads are added as we want the most recent to be left most.
> 
> Belay that comment I misread what you were saying, I will update patch with
> suggested insertion.

Sorry about that, I hope this is more to your liking.

[Peter Kasting, 2014-05-22 18:23:14]

LGTM

https://codereview.chromium.org/274273002/diff/60001/chrome/browser/ui/views/...
File chrome/browser/ui/views/download/download_shelf_view.cc (right):

https://codereview.chromium.org/274273002/diff/60001/chrome/browser/ui/views/...
chrome/browser/ui/views/download/download_shelf_view.cc:118: // Insert the new
view at index 0 to maintain a newest to oldest tab order.
Nit: How about:

Insert the new view as the first child, so the logical child order matches the
visual order.  This ensures that e.g. tabbing through downloads happens in the
order users would expect.

https://codereview.chromium.org/274273002/diff/60001/chrome/browser/ui/views/...
chrome/browser/ui/views/download/download_shelf_view.cc:155:
download_views_[download_views_.size() - 1];
Nit: download_views_.back()

[Harry McCleave, 2014-05-23 20:34:24]

The CQ bit was checked by harrym@chromium.org

[commit-bot: I haz the power, 2014-05-23 20:35:24]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/harrym@chromium.org/274273002/100001

[commit-bot: I haz the power, 2014-05-24 01:15:43]

Change committed as 272667