The delay is unsurprising and I don't think is actually new/different, it's just more noticeable with the shift in the UI, which is definitely a bug.

The shift may have been addressed with recent changes. We should check the behavior after other fixes in the CQ land (e.g.  issue 752842 ).

This will affect 61.

Yes, the delay is nothing new. Just more noticeable now that the size of the camera and image preview is different. The delay should actually be a bit less in canary as I reduced the size of the captured image some.

The reason for the delay is some bouncing back and forth to the browser process where we decode and then re-encode the image afaict. We could make this fast by simply adjusting the settings UI right away using the captured image. Maybe even animate from the camera to the image preview.

UX signed off on the smaller preview for both profile picture and images. We might revisit that if needed. Maybe keep the animated images small and the profile picture larger.

The reason it scrolls is because it tries to make sure the new selected image on the right is in view. We can improve that and not scroll if already in view.

Are we doing that explicitly somewhere?

For  issue 752034  I used scrollIntoViewIfNeeded(), which provides better behavior than scrollIntoView() or other options, FWIW.

(Note:  issue 752034  is not in 61 yet)

Just tested the behavior in ToT.

1. The size change between the camera and 'old image' view is pretty jarring. I definitely think we should scale down the camera to match everything else.

2. The scrolling occurs whenever we focus the list of images. This is because we opted to fill the page with images instead of using a fixed size scrolling container (which is what we do in the OOBE dialog), so the default HTML behavior is to fit as much of the element on the screen as possible, causing the page to scroll.

I can't think of a straightforward fix here, other than to limit the picture list container size and scroll it. I am open to suggestions.

elizabethchiu@ - thoughts/suggestions?

Ah, it tries to fit as much of the picture list as possible on screen. Got it.

I tried limiting the picture list container size and scroll it as part of the UI changes I made and found that it looked really weird when it was limited for what seemed like no good reason. If we limit it based on the size of the settings window then it might look Ok.

I think we should land something like https://chromium-review.googlesource.com/c/619168 for M61. I think that improves the UX significantly.

I've been thinking more about how we can make the transition from camera to image preview more seamless. One solution is of course to make the size the same either by increasing size of the preview or reducing the size of the camera. That's something we can easily do in the M61 time frame.

Long term, maybe it would be better to have the camera show up as a dialog (like the file chooser) instead of having it built into the change picture page. Wdyt?

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

commit 878a62a3d029bf1a9105095c256641d98a511c7f
Author: David Reveman <reveman@chromium.org>
Date: Sat Aug 19 00:25:57 2017

Change Picture: Move vertical scrolling from page to picture list.

Allow picture list to be scrolled instead of the page. The visible
height of the picture list still match the viewport and window.

BUG= 755929 

Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I14360fe669eda889a6fda25ab84e44204e2e455a
Reviewed-on: https://chromium-review.googlesource.com/619168
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: David Reveman <reveman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495763}
[modify] https://crrev.com/878a62a3d029bf1a9105095c256641d98a511c7f/chrome/browser/resources/settings/people_page/change_picture.html

-> reveman@ - this should be fixed now?

I think the UX is much better now that the scrolling has been fixed. We might also want https://chromium-review.googlesource.com/624778 to reduce the delay after taking a photo.

I'm not happy about the change in size from photo to preview and I'm exploring some options there. I think something close to what we have now will have to do for 61 though so I'll close this after the above mentioned patch has landed and open new bugs for future UI adjustments.

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

commit 0a7aafaae4eb1d726d7564d4d63960a73c36594d
Author: David Reveman <reveman@chromium.org>
Date: Tue Aug 22 04:22:14 2017

Change Picture: Reduce delay when taking photo

Update picture list immediately after taking a photo instead of
waiting for the browser to do its decode-re-encode dance. This
is consistent with what the oobe UI already does.

Bug:  755929 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I4bcd94a6bddfd2ca5dfbcd3a4faf746fa5af96a4
Reviewed-on: https://chromium-review.googlesource.com/624778
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: David Reveman <reveman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496212}
[modify] https://crrev.com/0a7aafaae4eb1d726d7564d4d63960a73c36594d/chrome/browser/resources/settings/people_page/change_picture.js

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

commit 47d357f456e036dffa09b6db6bd09837058b8e80
Author: David Reveman <reveman@chromium.org>
Date: Fri Aug 25 08:03:21 2017

Change Picture: Move vertical scrolling from page to picture list.

Allow picture list to be scrolled instead of the page. The visible
height of the picture list still match the viewport and window.

BUG= 755929 
TBR=reveman@chromium.org

(cherry picked from commit 878a62a3d029bf1a9105095c256641d98a511c7f)

Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I14360fe669eda889a6fda25ab84e44204e2e455a
Reviewed-on: https://chromium-review.googlesource.com/619168
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: David Reveman <reveman@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#495763}
Reviewed-on: https://chromium-review.googlesource.com/635008
Reviewed-by: David Reveman <reveman@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#880}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/47d357f456e036dffa09b6db6bd09837058b8e80/chrome/browser/resources/settings/people_page/change_picture.html

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

commit 96814592bfb715b0a6aa9d5047c5323dd30a471b
Author: David Reveman <reveman@chromium.org>
Date: Fri Aug 25 08:13:24 2017

Change Picture: Reduce delay when taking photo

Update picture list immediately after taking a photo instead of
waiting for the browser to do its decode-re-encode dance. This
is consistent with what the oobe UI already does.

TBR=reveman@chromium.org

(cherry picked from commit 0a7aafaae4eb1d726d7564d4d63960a73c36594d)

Bug:  755929 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I4bcd94a6bddfd2ca5dfbcd3a4faf746fa5af96a4
Reviewed-on: https://chromium-review.googlesource.com/624778
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: David Reveman <reveman@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#496212}
Reviewed-on: https://chromium-review.googlesource.com/634986
Reviewed-by: David Reveman <reveman@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#882}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/96814592bfb715b0a6aa9d5047c5323dd30a471b/chrome/browser/resources/settings/people_page/change_picture.js

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

commit 682de0452a227e3ed066e30419916d62df9893f0
Author: David Reveman <reveman@chromium.org>
Date: Wed Aug 30 19:50:28 2017

Change Picture: Fix layout issues.

This makes the layout not depend on a specific font size to align
to the viewport properly and it improves handling of small window
sizes significantly.

Bug:  755929 
Test: manually changed window and font size
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I896cb184ca95cbcb9d3191f4d61f49d6eaa08cc5
Reviewed-on: https://chromium-review.googlesource.com/643551
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: David Reveman <reveman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498575}
[modify] https://crrev.com/682de0452a227e3ed066e30419916d62df9893f0/chrome/browser/resources/settings/people_page/change_picture.html

This bug requires manual review: We are only 5 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Approving merge to M61.

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

commit 6683212649f437f9078c7c1ebe34d4f0094364c5
Author: David Reveman <reveman@chromium.org>
Date: Thu Aug 31 00:24:29 2017

Change Picture: Fix layout issues.

This makes the layout not depend on a specific font size to align
to the viewport properly and it improves handling of small window
sizes significantly.

TBR=reveman@chromium.org

(cherry picked from commit 682de0452a227e3ed066e30419916d62df9893f0)

Bug:  755929 
Test: manually changed window and font size
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I896cb184ca95cbcb9d3191f4d61f49d6eaa08cc5
Reviewed-on: https://chromium-review.googlesource.com/643551
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: David Reveman <reveman@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#498575}
Reviewed-on: https://chromium-review.googlesource.com/644360
Reviewed-by: David Reveman <reveman@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#1021}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/6683212649f437f9078c7c1ebe34d4f0094364c5/chrome/browser/resources/settings/people_page/change_picture.html

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

commit 6a69033c6082f3fc61ffbe7876eb88a83e44d08b
Author: David Reveman <reveman@chromium.org>
Date: Tue Sep 05 22:11:17 2017

Change Picture: Improve layout more.

Takes font-size based padding for header into account and also
sets top position for container to ensure that viewport alignment
is still correct even if assumptions about header height are not
perfect.

Bug:  761301 ,  755929 
Test: verified layout is good at all font sizes
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ifbcf8301571fb6313829ef39b1c74eb6926550e3
Reviewed-on: https://chromium-review.googlesource.com/648111
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: David Reveman <reveman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499767}
[modify] https://crrev.com/6a69033c6082f3fc61ffbe7876eb88a83e44d08b/chrome/browser/resources/settings/people_page/change_picture.html

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

commit a1391f9e9f2528d100ccc1f14f620f3efaaacfbc
Author: David Reveman <reveman@chromium.org>
Date: Fri Sep 08 01:07:07 2017

Change Picture: Improve layout more.

Takes font-size based padding for header into account and also
sets top position for container to ensure that viewport alignment
is still correct even if assumptions about header height are not
perfect.

TBR=reveman@chromium.org

(cherry picked from commit 6a69033c6082f3fc61ffbe7876eb88a83e44d08b)

Bug:  761301 ,  755929 
Test: verified layout is good at all font sizes
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ifbcf8301571fb6313829ef39b1c74eb6926550e3
Reviewed-on: https://chromium-review.googlesource.com/648111
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: David Reveman <reveman@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#499767}
Reviewed-on: https://chromium-review.googlesource.com/656699
Reviewed-by: David Reveman <reveman@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#81}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/a1391f9e9f2528d100ccc1f14f620f3efaaacfbc/chrome/browser/resources/settings/people_page/change_picture.html

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

commit 85cd459a0459400a4779184b5d6d2572c49cb1ac
Author: David Reveman <reveman@chromium.org>
Date: Fri Sep 08 01:08:43 2017

Change Picture: Improve layout more.

Takes font-size based padding for header into account and also
sets top position for container to ensure that viewport alignment
is still correct even if assumptions about header height are not
perfect.

TBR=reveman@chromium.org

(cherry picked from commit 6a69033c6082f3fc61ffbe7876eb88a83e44d08b)

Bug:  761301 ,  755929 
Test: verified layout is good at all font sizes
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ifbcf8301571fb6313829ef39b1c74eb6926550e3
Reviewed-on: https://chromium-review.googlesource.com/648111
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: David Reveman <reveman@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#499767}
Reviewed-on: https://chromium-review.googlesource.com/656718
Reviewed-by: David Reveman <reveman@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#1142}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/85cd459a0459400a4779184b5d6d2572c49cb1ac/chrome/browser/resources/settings/people_page/change_picture.html