Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2898283002:
Multiple user pods implementation for new login screen (Closed)
|
Created:
3 years, 7 months ago by Wenzhao (Colin) Zang Modified:
3 years, 7 months ago CC:
chromium-reviews, alemate+watch_chromium.org, achuith+watch_chromium.org, rkc Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionMultiple user pods implementation for new login screen
The new login screen for multiple user pods involves creating small
pods and places them on a scroll bar.
The ‘small pod’ is implemented as an ‘add-on’ to the existing user pod
template, instead of being a new prototype. Pods can easily transform
itself among the ‘large’, ‘small’ and ‘extra small’ states, by toggling
the visibility of corresponding parts.
1) Pod placement works as follows:
Upon receiving the user list from backend, create a new pod for each user
(in rebuildPods()), append them to the pow row as children, and call
initialize(). This part remains the same with the old implementation, and
at this point there is no need to determine what size the pod should be
shown.
rebuildPods() calls placePods(). placePods() first determines the ‘main
pod’ (the one displayed as the large pod), and append all other pods to a
‘small pods container’, because they should be displayed as small pods.
This is done in appendPodsToParents().
The actual placement process starts here:
it’s done by: placeSinglePod_(), placePodsOnPodRow_(),
placePodsOnContainer_() or placePodsOnScrollableContainer_().
The placePods() function decides which one to call based on the number of
users we have. There’re lots of details in the placement process but the
general logic is simple, because we do not have to determine the best
numbers of rows and columns and can just follow the spec instead.
One special handling is: when scrollable container is shown it should
occupy the full screen, and a gradient mask is applied to avoid blocking
the header bar. The z-indexes should be well selected.
2) User removal works as follows:
A pod should always be switched to the main pod before being removed
(except the trivial 2-user case). Therefore after removing it from its
parent (the pod row), the main pod is set to null.
We call placePods(), and the following works exactly the same with the 1)
scenario. This is because placePods() will determine the main pod and re-
append the pods to different parents if necessary.
3) Window resizing works as follows:
Once there is a window resizing event, call placePods() directly. The
difference with the above two scenarios is that the main pod is not null
when window resizing happens. Therefore, placePods() will skip the main
pod selection and re-appending children process, and start the actual
placement directly.
4) Click event:
The only click event on small pods is to trigger the switch between the
small pod and the large pod. In the click event handler, we first find
out if it’s for a small pod, and if it is, simply switch it with the main
pod by switching the parents as well as positions / other relevant styles
(handled by changeMainPod()).
Please note:
We do not call placePods() because it may result in reordering the pods.
It’s OK that the pods are not in LRU order temporarily.
BUG=718159
Review-Url: https://codereview.chromium.org/2898283002
Cr-Commit-Position: refs/heads/master@{#474889}
Committed: https://chromium.googlesource.com/chromium/src/+/c1604bc83510da9806cb1c00d3351d8fcb7eb24a
Patch Set 1 #
Total comments: 21
Patch Set 2 : Add more comments #Patch Set 3 : Simplify the pod placement process and make it adaptive to the virtual keyboard #
Total comments: 24
Patch Set 4 : Address comments and add corner case handling for virtual keyboard #
Total comments: 4
Patch Set 5 : Address comments #
Messages
Total messages: 33 (17 generated)
Description was changed from ========== Login screen redesign (CL 3) BUG=718159 ========== to ========== Login screen redesign (CL 3) Handling of multiple user pods (small pods, scroll bar etc.) Please refer to 'Step 3' section of this doc for more information: https://docs.google.com/document/d/1gILlP9VyBvfikcFZifkxZT4dEdZ1nLFV__X6RHT9Z... BUG=718159 ==========
wzang@chromium.org changed reviewers: + alemate@chromium.org, jdufault@chromium.org, xiyuan@chromium.org
xiyuan@, jdufault@, alemate@, PTAL. This CL might be too long and refer to https://docs.google.com/document/d/1gILlP9VyBvfikcFZifkxZT4dEdZ1nLFV__X6RHT9Z... or ping me for any questions. Thanks so much.
rkc@chromium.org changed reviewers: + rkc@chromium.org
We need, a.) a better title b.) more information in the CL description. Referring people to a section in a design doc in a CL description is not okay. The design doc can change and doesn't always describe exactly what the CL is doing. The CL title and description should give a clear idea of "what" we're doing, at a minimum.
General approach seems fine, it is very challenging to determine which code paths do what though. Can you add comments codifying what you know right now? That should make picking up the code again faster in a month, I suspect there are tons of little details here that are easily forgotten. https://codereview.chromium.org/2898283002/diff/1/ui/login/account_picker/md_... File ui/login/account_picker/md_user_pod_row.css (right): https://codereview.chromium.org/2898283002/diff/1/ui/login/account_picker/md_... ui/login/account_picker/md_user_pod_row.css:1018: .large-pod { As FYI, I've been using the naming convention 'few', 'some', 'many' because it avoids the 'small' / 'extra-small' name which is kind of strange. Though large/small/extra-small is fine. https://codereview.chromium.org/2898283002/diff/1/ui/login/account_picker/md_... File ui/login/account_picker/md_user_pod_row.js (right): https://codereview.chromium.org/2898283002/diff/1/ui/login/account_picker/md_... ui/login/account_picker/md_user_pod_row.js:1153: this.querySelector('.action-box-remove-user-warning-history'), revert? https://codereview.chromium.org/2898283002/diff/1/ui/login/account_picker/md_... ui/login/account_picker/md_user_pod_row.js:1202: this.smallPodImageElement.src = 'chrome://userimage/' + Try to avoid redefining (non-trivial) strings multiple times. var imageSrc = ... this.imageElement.src = imageSrc this.smallPodImageElement.src = imageSrc https://codereview.chromium.org/2898283002/diff/1/ui/login/account_picker/md_... ui/login/account_picker/md_user_pod_row.js:2378: $('pod-row').changeMainPod_(this); changeMainPod_ seems like it should be a public (ie, no _) function Maybe rename to setFocusedPod? Or are you trying to avoid using the name focus? https://codereview.chromium.org/2898283002/diff/1/ui/login/account_picker/md_... ui/login/account_picker/md_user_pod_row.js:2383: this.parentNode.setActivatedPod(this, e); If you remove these two calls, does anything break? https://codereview.chromium.org/2898283002/diff/1/ui/login/account_picker/md_... ui/login/account_picker/md_user_pod_row.js:3473: onWindowResize: function() { Did you test with virtual keyboard? https://codereview.chromium.org/2898283002/diff/1/ui/login/account_picker/md_... ui/login/account_picker/md_user_pod_row.js:3577: for (var i = 0, pod; pod = pods[i]; ++i) { for (var pod of pods)? https://codereview.chromium.org/2898283002/diff/1/ui/login/account_picker/md_... ui/login/account_picker/md_user_pod_row.js:3581: if (pods.length <= 2) { Add comment on the 2 / make it a constant https://codereview.chromium.org/2898283002/diff/1/ui/login/account_picker/md_... ui/login/account_picker/md_user_pod_row.js:3636: if ((pods.length > 6 && !this.isPortraitMode()) || constant for 6 and 10 https://codereview.chromium.org/2898283002/diff/1/ui/login/account_picker/md_... ui/login/account_picker/md_user_pod_row.js:3851: if (pod.isInExtraSmallForm()) { Instead of having separate functions for isExtraSmall, isSmall, it may be easier to have an enum, ie, this.mainPod_.setPodStyle(pod.getPodStyle())
https://codereview.chromium.org/2898283002/diff/1/ui/login/account_picker/md_... File ui/login/account_picker/md_user_pod_row.css (right): https://codereview.chromium.org/2898283002/diff/1/ui/login/account_picker/md_... ui/login/account_picker/md_user_pod_row.css:1018: .large-pod { On 2017/05/23 20:34:32, jdufault wrote: > As FYI, I've been using the naming convention 'few', 'some', 'many' because it > avoids the 'small' / 'extra-small' name which is kind of strange. > > Though large/small/extra-small is fine. These could be defined functionally instead of visually? For example, we should have a "focused-pod" and "inactive-pod" (just suggestions) or something along those lines. We should try to stay away from using terms that are inherently vague like, large, small or even few, some or many.
Description was changed from ========== Login screen redesign (CL 3) Handling of multiple user pods (small pods, scroll bar etc.) Please refer to 'Step 3' section of this doc for more information: https://docs.google.com/document/d/1gILlP9VyBvfikcFZifkxZT4dEdZ1nLFV__X6RHT9Z... BUG=718159 ========== to ========== Multiple user pods implementation for new login screen Handling of multiple user pods (small pods, scroll bar etc.) Please refer to 'Step 3' section of this doc for more information: The goal is to match page 2 of the spec here. The ‘small pod’ is implemented as an ‘add-on’ to the existing user pod template, instead of being a new prototype. Pods can easily transform itself among the ‘large’, ‘small’ and ‘extra small’ states, by toggling the visibility of corresponding parts. 1) Pod placement works as follows: Upon receiving the user list from backend, create a new pod for each user (in rebuildPods()), append them to the pow row as children, and call initialize(). This part remain the same with the old implementation, and at this point there is no need to determine what size the pod should be shown. rebuildPods() calls placePods(). placePods() first determines the ‘main pod’ (the one displayed as the large pod), and append all other pods to a ‘small pods container’, because they should be displayed as small pods. This is done in appendPodsToParents() in line 3554. The actual placement process starts here: it’s is done by: placeSinglePod(), placeDoublePods(), placePodsOnContainer() or placePodsOnScrollableContainer(). The placePods() function decides which one to call based on the number of users we have. There’re lots of details in the placement process but the general logic is simple, because we do not have to determine the best numbers of rows and columns and can just follow the spec instead. One special handling is: when scrollable container is shown it should occupy the full screen, and a gradient mask is applied to avoid blocking the header bar. The z-indexes should be well selected. 2) User removal works as follows: A pod should always be switched to the main pod before being removed (except the trivial 2-user case). Therefore after removing it from its parent (the pod row), the main pod is set to null. We call placePods(), and the following works exactly the same with the 1) scenario. This is because placePods() will determine the main pod and re-append the pods to different parents if necessary. 3) Window resizing works as follows: Once there is a window resizing event, call placePods() directly. The difference with the above two scenarios is that the main pod is not null when window resizing happens. Therefore, placePods() will skip the main pod selection and re-appending children process, and start the actual placement directly. 4) Click event: The only click event on small pods is to trigger the switch between the small pod and the large pod. In the click event handler, we first find out if it’s for a small pod, and if it is, simply switch it with the main pod by switching the parents as well as positions / other relevant styles (handled by changeMainPod()). Please note: We do not call placePods() because it may result in reordering the pods. It’s OK that the pods are not in LRU order temporarily. This is added to the existing click handler so we do not need to register a new one. BUG=718159 ==========
Description was changed from ========== Multiple user pods implementation for new login screen Handling of multiple user pods (small pods, scroll bar etc.) Please refer to 'Step 3' section of this doc for more information: The goal is to match page 2 of the spec here. The ‘small pod’ is implemented as an ‘add-on’ to the existing user pod template, instead of being a new prototype. Pods can easily transform itself among the ‘large’, ‘small’ and ‘extra small’ states, by toggling the visibility of corresponding parts. 1) Pod placement works as follows: Upon receiving the user list from backend, create a new pod for each user (in rebuildPods()), append them to the pow row as children, and call initialize(). This part remain the same with the old implementation, and at this point there is no need to determine what size the pod should be shown. rebuildPods() calls placePods(). placePods() first determines the ‘main pod’ (the one displayed as the large pod), and append all other pods to a ‘small pods container’, because they should be displayed as small pods. This is done in appendPodsToParents() in line 3554. The actual placement process starts here: it’s is done by: placeSinglePod(), placeDoublePods(), placePodsOnContainer() or placePodsOnScrollableContainer(). The placePods() function decides which one to call based on the number of users we have. There’re lots of details in the placement process but the general logic is simple, because we do not have to determine the best numbers of rows and columns and can just follow the spec instead. One special handling is: when scrollable container is shown it should occupy the full screen, and a gradient mask is applied to avoid blocking the header bar. The z-indexes should be well selected. 2) User removal works as follows: A pod should always be switched to the main pod before being removed (except the trivial 2-user case). Therefore after removing it from its parent (the pod row), the main pod is set to null. We call placePods(), and the following works exactly the same with the 1) scenario. This is because placePods() will determine the main pod and re-append the pods to different parents if necessary. 3) Window resizing works as follows: Once there is a window resizing event, call placePods() directly. The difference with the above two scenarios is that the main pod is not null when window resizing happens. Therefore, placePods() will skip the main pod selection and re-appending children process, and start the actual placement directly. 4) Click event: The only click event on small pods is to trigger the switch between the small pod and the large pod. In the click event handler, we first find out if it’s for a small pod, and if it is, simply switch it with the main pod by switching the parents as well as positions / other relevant styles (handled by changeMainPod()). Please note: We do not call placePods() because it may result in reordering the pods. It’s OK that the pods are not in LRU order temporarily. This is added to the existing click handler so we do not need to register a new one. BUG=718159 ========== to ========== Multiple user pods implementation for new login screen The new login screen for multiple user pods involve creating small pods and place them on a scroll bar. The ‘small pod’ is implemented as an ‘add-on’ to the existing user pod template, instead of being a new prototype. Pods can easily transform itself among the ‘large’, ‘small’ and ‘extra small’ states, by toggling the visibility of corresponding parts. 1) Pod placement works as follows: Upon receiving the user list from backend, create a new pod for each user (in rebuildPods()), append them to the pow row as children, and call initialize(). This part remain the same with the old implementation, and at this point there is no need to determine what size the pod should be shown. rebuildPods() calls placePods(). placePods() first determines the ‘main pod’ (the one displayed as the large pod), and append all other pods to a ‘small pods container’, because they should be displayed as small pods. This is done in appendPodsToParents() in line 3554. The actual placement process starts here: it’s is done by: placeSinglePod(), placeDoublePods(), placePodsOnContainer() or placePodsOnScrollableContainer(). The placePods() function decides which one to call based on the number of users we have. There’re lots of details in the placement process but the general logic is simple, because we do not have to determine the best numbers of rows and columns and can just follow the spec instead. One special handling is: when scrollable container is shown it should occupy the full screen, and a gradient mask is applied to avoid blocking the header bar. The z-indexes should be well selected. 2) User removal works as follows: A pod should always be switched to the main pod before being removed (except the trivial 2-user case). Therefore after removing it from its parent (the pod row), the main pod is set to null. We call placePods(), and the following works exactly the same with the 1) scenario. This is because placePods() will determine the main pod and re- append the pods to different parents if necessary. 3) Window resizing works as follows: Once there is a window resizing event, call placePods() directly. The difference with the above two scenarios is that the main pod is not null when window resizing happens. Therefore, placePods() will skip the main pod selection and re-appending children process, and start the actual placement directly. 4) Click event: The only click event on small pods is to trigger the switch between the small pod and the large pod. In the click event handler, we first find out if it’s for a small pod, and if it is, simply switch it with the main pod by switching the parents as well as positions / other relevant styles (handled by changeMainPod()). Please note: We do not call placePods() because it may result in reordering the pods. It’s OK that the pods are not in LRU order temporarily. This is added to the existing click handler so we do not need to register a new one. BUG=718159 ==========
Description was changed from ========== Multiple user pods implementation for new login screen The new login screen for multiple user pods involve creating small pods and place them on a scroll bar. The ‘small pod’ is implemented as an ‘add-on’ to the existing user pod template, instead of being a new prototype. Pods can easily transform itself among the ‘large’, ‘small’ and ‘extra small’ states, by toggling the visibility of corresponding parts. 1) Pod placement works as follows: Upon receiving the user list from backend, create a new pod for each user (in rebuildPods()), append them to the pow row as children, and call initialize(). This part remain the same with the old implementation, and at this point there is no need to determine what size the pod should be shown. rebuildPods() calls placePods(). placePods() first determines the ‘main pod’ (the one displayed as the large pod), and append all other pods to a ‘small pods container’, because they should be displayed as small pods. This is done in appendPodsToParents() in line 3554. The actual placement process starts here: it’s is done by: placeSinglePod(), placeDoublePods(), placePodsOnContainer() or placePodsOnScrollableContainer(). The placePods() function decides which one to call based on the number of users we have. There’re lots of details in the placement process but the general logic is simple, because we do not have to determine the best numbers of rows and columns and can just follow the spec instead. One special handling is: when scrollable container is shown it should occupy the full screen, and a gradient mask is applied to avoid blocking the header bar. The z-indexes should be well selected. 2) User removal works as follows: A pod should always be switched to the main pod before being removed (except the trivial 2-user case). Therefore after removing it from its parent (the pod row), the main pod is set to null. We call placePods(), and the following works exactly the same with the 1) scenario. This is because placePods() will determine the main pod and re- append the pods to different parents if necessary. 3) Window resizing works as follows: Once there is a window resizing event, call placePods() directly. The difference with the above two scenarios is that the main pod is not null when window resizing happens. Therefore, placePods() will skip the main pod selection and re-appending children process, and start the actual placement directly. 4) Click event: The only click event on small pods is to trigger the switch between the small pod and the large pod. In the click event handler, we first find out if it’s for a small pod, and if it is, simply switch it with the main pod by switching the parents as well as positions / other relevant styles (handled by changeMainPod()). Please note: We do not call placePods() because it may result in reordering the pods. It’s OK that the pods are not in LRU order temporarily. This is added to the existing click handler so we do not need to register a new one. BUG=718159 ========== to ========== Multiple user pods implementation for new login screen The new login screen for multiple user pods involve creating small pods and place them on a scroll bar. The ‘small pod’ is implemented as an ‘add-on’ to the existing user pod template, instead of being a new prototype. Pods can easily transform itself among the ‘large’, ‘small’ and ‘extra small’ states, by toggling the visibility of corresponding parts. 1) Pod placement works as follows: Upon receiving the user list from backend, create a new pod for each user (in rebuildPods()), append them to the pow row as children, and call initialize(). This part remain the same with the old implementation, and at this point there is no need to determine what size the pod should be shown. rebuildPods() calls placePods(). placePods() first determines the ‘main pod’ (the one displayed as the large pod), and append all other pods to a ‘small pods container’, because they should be displayed as small pods. This is done in appendPodsToParents() in line 3554. The actual placement process starts here: it’s is done by: placeSinglePod(), placeDoublePods(), placePodsOnContainer() or placePodsOnScrollableContainer(). The placePods() function decides which one to call based on the number of users we have. There’re lots of details in the placement process but the general logic is simple, because we do not have to determine the best numbers of rows and columns and can just follow the spec instead. One special handling is: when scrollable container is shown it should occupy the full screen, and a gradient mask is applied to avoid blocking the header bar. The z-indexes should be well selected. 2) User removal works as follows: A pod should always be switched to the main pod before being removed (except the trivial 2-user case). Therefore after removing it from its parent (the pod row), the main pod is set to null. We call placePods(), and the following works exactly the same with the 1) scenario. This is because placePods() will determine the main pod and re- append the pods to different parents if necessary. 3) Window resizing works as follows: Once there is a window resizing event, call placePods() directly. The difference with the above two scenarios is that the main pod is not null when window resizing happens. Therefore, placePods() will skip the main pod selection and re-appending children process, and start the actual placement directly. 4) Click event: The only click event on small pods is to trigger the switch between the small pod and the large pod. In the click event handler, we first find out if it’s for a small pod, and if it is, simply switch it with the main pod by switching the parents as well as positions / other relevant styles (handled by changeMainPod()). Please note: We do not call placePods() because it may result in reordering the pods. It’s OK that the pods are not in LRU order temporarily. This is added to the existing click handler so we do not need to register a new one. BUG=718159 ==========
Description was changed from ========== Multiple user pods implementation for new login screen The new login screen for multiple user pods involve creating small pods and place them on a scroll bar. The ‘small pod’ is implemented as an ‘add-on’ to the existing user pod template, instead of being a new prototype. Pods can easily transform itself among the ‘large’, ‘small’ and ‘extra small’ states, by toggling the visibility of corresponding parts. 1) Pod placement works as follows: Upon receiving the user list from backend, create a new pod for each user (in rebuildPods()), append them to the pow row as children, and call initialize(). This part remain the same with the old implementation, and at this point there is no need to determine what size the pod should be shown. rebuildPods() calls placePods(). placePods() first determines the ‘main pod’ (the one displayed as the large pod), and append all other pods to a ‘small pods container’, because they should be displayed as small pods. This is done in appendPodsToParents() in line 3554. The actual placement process starts here: it’s is done by: placeSinglePod(), placeDoublePods(), placePodsOnContainer() or placePodsOnScrollableContainer(). The placePods() function decides which one to call based on the number of users we have. There’re lots of details in the placement process but the general logic is simple, because we do not have to determine the best numbers of rows and columns and can just follow the spec instead. One special handling is: when scrollable container is shown it should occupy the full screen, and a gradient mask is applied to avoid blocking the header bar. The z-indexes should be well selected. 2) User removal works as follows: A pod should always be switched to the main pod before being removed (except the trivial 2-user case). Therefore after removing it from its parent (the pod row), the main pod is set to null. We call placePods(), and the following works exactly the same with the 1) scenario. This is because placePods() will determine the main pod and re- append the pods to different parents if necessary. 3) Window resizing works as follows: Once there is a window resizing event, call placePods() directly. The difference with the above two scenarios is that the main pod is not null when window resizing happens. Therefore, placePods() will skip the main pod selection and re-appending children process, and start the actual placement directly. 4) Click event: The only click event on small pods is to trigger the switch between the small pod and the large pod. In the click event handler, we first find out if it’s for a small pod, and if it is, simply switch it with the main pod by switching the parents as well as positions / other relevant styles (handled by changeMainPod()). Please note: We do not call placePods() because it may result in reordering the pods. It’s OK that the pods are not in LRU order temporarily. This is added to the existing click handler so we do not need to register a new one. BUG=718159 ========== to ========== Multiple user pods implementation for new login screen The new login screen for multiple user pods involve creating small pods and place them on a scroll bar. The ‘small pod’ is implemented as an ‘add-on’ to the existing user pod template, instead of being a new prototype. Pods can easily transform itself among the ‘large’, ‘small’ and ‘extra small’ states, by toggling the visibility of corresponding parts. 1) Pod placement works as follows: Upon receiving the user list from backend, create a new pod for each user (in rebuildPods()), append them to the pow row as children, and call initialize(). This part remain the same with the old implementation, and at this point there is no need to determine what size the pod should be shown. rebuildPods() calls placePods(). placePods() first determines the ‘main pod’ (the one displayed as the large pod), and append all other pods to a ‘small pods container’, because they should be displayed as small pods. This is done in appendPodsToParents() in line 3554. The actual placement process starts here: it’s done by: placeSinglePod(), placeDoublePods(), placePodsOnContainer() or placePodsOnScrollableContainer(). The placePods() function decides which one to call based on the number of users we have. There’re lots of details in the placement process but the general logic is simple, because we do not have to determine the best numbers of rows and columns and can just follow the spec instead. One special handling is: when scrollable container is shown it should occupy the full screen, and a gradient mask is applied to avoid blocking the header bar. The z-indexes should be well selected. 2) User removal works as follows: A pod should always be switched to the main pod before being removed (except the trivial 2-user case). Therefore after removing it from its parent (the pod row), the main pod is set to null. We call placePods(), and the following works exactly the same with the 1) scenario. This is because placePods() will determine the main pod and re- append the pods to different parents if necessary. 3) Window resizing works as follows: Once there is a window resizing event, call placePods() directly. The difference with the above two scenarios is that the main pod is not null when window resizing happens. Therefore, placePods() will skip the main pod selection and re-appending children process, and start the actual placement directly. 4) Click event: The only click event on small pods is to trigger the switch between the small pod and the large pod. In the click event handler, we first find out if it’s for a small pod, and if it is, simply switch it with the main pod by switching the parents as well as positions / other relevant styles (handled by changeMainPod()). Please note: We do not call placePods() because it may result in reordering the pods. It’s OK that the pods are not in LRU order temporarily. This is added to the existing click handler so we do not need to register a new one. BUG=718159 ==========
Description was changed from ========== Multiple user pods implementation for new login screen The new login screen for multiple user pods involve creating small pods and place them on a scroll bar. The ‘small pod’ is implemented as an ‘add-on’ to the existing user pod template, instead of being a new prototype. Pods can easily transform itself among the ‘large’, ‘small’ and ‘extra small’ states, by toggling the visibility of corresponding parts. 1) Pod placement works as follows: Upon receiving the user list from backend, create a new pod for each user (in rebuildPods()), append them to the pow row as children, and call initialize(). This part remain the same with the old implementation, and at this point there is no need to determine what size the pod should be shown. rebuildPods() calls placePods(). placePods() first determines the ‘main pod’ (the one displayed as the large pod), and append all other pods to a ‘small pods container’, because they should be displayed as small pods. This is done in appendPodsToParents() in line 3554. The actual placement process starts here: it’s done by: placeSinglePod(), placeDoublePods(), placePodsOnContainer() or placePodsOnScrollableContainer(). The placePods() function decides which one to call based on the number of users we have. There’re lots of details in the placement process but the general logic is simple, because we do not have to determine the best numbers of rows and columns and can just follow the spec instead. One special handling is: when scrollable container is shown it should occupy the full screen, and a gradient mask is applied to avoid blocking the header bar. The z-indexes should be well selected. 2) User removal works as follows: A pod should always be switched to the main pod before being removed (except the trivial 2-user case). Therefore after removing it from its parent (the pod row), the main pod is set to null. We call placePods(), and the following works exactly the same with the 1) scenario. This is because placePods() will determine the main pod and re- append the pods to different parents if necessary. 3) Window resizing works as follows: Once there is a window resizing event, call placePods() directly. The difference with the above two scenarios is that the main pod is not null when window resizing happens. Therefore, placePods() will skip the main pod selection and re-appending children process, and start the actual placement directly. 4) Click event: The only click event on small pods is to trigger the switch between the small pod and the large pod. In the click event handler, we first find out if it’s for a small pod, and if it is, simply switch it with the main pod by switching the parents as well as positions / other relevant styles (handled by changeMainPod()). Please note: We do not call placePods() because it may result in reordering the pods. It’s OK that the pods are not in LRU order temporarily. This is added to the existing click handler so we do not need to register a new one. BUG=718159 ========== to ========== Multiple user pods implementation for new login screen The new login screen for multiple user pods involve creating small pods and place them on a scroll bar. The ‘small pod’ is implemented as an ‘add-on’ to the existing user pod template, instead of being a new prototype. Pods can easily transform itself among the ‘large’, ‘small’ and ‘extra small’ states, by toggling the visibility of corresponding parts. 1) Pod placement works as follows: Upon receiving the user list from backend, create a new pod for each user (in rebuildPods()), append them to the pow row as children, and call initialize(). This part remains the same with the old implementation, and at this point there is no need to determine what size the pod should be shown. rebuildPods() calls placePods(). placePods() first determines the ‘main pod’ (the one displayed as the large pod), and append all other pods to a ‘small pods container’, because they should be displayed as small pods. This is done in appendPodsToParents(). The actual placement process starts here: it’s done by: placeSinglePod(), placeDoublePods(), placePodsOnContainer() or placePodsOnScrollableContainer(). The placePods() function decides which one to call based on the number of users we have. There’re lots of details in the placement process but the general logic is simple, because we do not have to determine the best numbers of rows and columns and can just follow the spec instead. One special handling is: when scrollable container is shown it should occupy the full screen, and a gradient mask is applied to avoid blocking the header bar. The z-indexes should be well selected. 2) User removal works as follows: A pod should always be switched to the main pod before being removed (except the trivial 2-user case). Therefore after removing it from its parent (the pod row), the main pod is set to null. We call placePods(), and the following works exactly the same with the 1) scenario. This is because placePods() will determine the main pod and re- append the pods to different parents if necessary. 3) Window resizing works as follows: Once there is a window resizing event, call placePods() directly. The difference with the above two scenarios is that the main pod is not null when window resizing happens. Therefore, placePods() will skip the main pod selection and re-appending children process, and start the actual placement directly. 4) Click event: The only click event on small pods is to trigger the switch between the small pod and the large pod. In the click event handler, we first find out if it’s for a small pod, and if it is, simply switch it with the main pod by switching the parents as well as positions / other relevant styles (handled by changeMainPod()). Please note: We do not call placePods() because it may result in reordering the pods. It’s OK that the pods are not in LRU order temporarily. This is added to the existing click handler so we do not need to register a new one. BUG=718159 ==========
Description was changed from ========== Multiple user pods implementation for new login screen The new login screen for multiple user pods involve creating small pods and place them on a scroll bar. The ‘small pod’ is implemented as an ‘add-on’ to the existing user pod template, instead of being a new prototype. Pods can easily transform itself among the ‘large’, ‘small’ and ‘extra small’ states, by toggling the visibility of corresponding parts. 1) Pod placement works as follows: Upon receiving the user list from backend, create a new pod for each user (in rebuildPods()), append them to the pow row as children, and call initialize(). This part remains the same with the old implementation, and at this point there is no need to determine what size the pod should be shown. rebuildPods() calls placePods(). placePods() first determines the ‘main pod’ (the one displayed as the large pod), and append all other pods to a ‘small pods container’, because they should be displayed as small pods. This is done in appendPodsToParents(). The actual placement process starts here: it’s done by: placeSinglePod(), placeDoublePods(), placePodsOnContainer() or placePodsOnScrollableContainer(). The placePods() function decides which one to call based on the number of users we have. There’re lots of details in the placement process but the general logic is simple, because we do not have to determine the best numbers of rows and columns and can just follow the spec instead. One special handling is: when scrollable container is shown it should occupy the full screen, and a gradient mask is applied to avoid blocking the header bar. The z-indexes should be well selected. 2) User removal works as follows: A pod should always be switched to the main pod before being removed (except the trivial 2-user case). Therefore after removing it from its parent (the pod row), the main pod is set to null. We call placePods(), and the following works exactly the same with the 1) scenario. This is because placePods() will determine the main pod and re- append the pods to different parents if necessary. 3) Window resizing works as follows: Once there is a window resizing event, call placePods() directly. The difference with the above two scenarios is that the main pod is not null when window resizing happens. Therefore, placePods() will skip the main pod selection and re-appending children process, and start the actual placement directly. 4) Click event: The only click event on small pods is to trigger the switch between the small pod and the large pod. In the click event handler, we first find out if it’s for a small pod, and if it is, simply switch it with the main pod by switching the parents as well as positions / other relevant styles (handled by changeMainPod()). Please note: We do not call placePods() because it may result in reordering the pods. It’s OK that the pods are not in LRU order temporarily. This is added to the existing click handler so we do not need to register a new one. BUG=718159 ========== to ========== Multiple user pods implementation for new login screen The new login screen for multiple user pods involve creating small pods and place them on a scroll bar. The ‘small pod’ is implemented as an ‘add-on’ to the existing user pod template, instead of being a new prototype. Pods can easily transform itself among the ‘large’, ‘small’ and ‘extra small’ states, by toggling the visibility of corresponding parts. 1) Pod placement works as follows: Upon receiving the user list from backend, create a new pod for each user (in rebuildPods()), append them to the pow row as children, and call initialize(). This part remains the same with the old implementation, and at this point there is no need to determine what size the pod should be shown. rebuildPods() calls placePods(). placePods() first determines the ‘main pod’ (the one displayed as the large pod), and append all other pods to a ‘small pods container’, because they should be displayed as small pods. This is done in appendPodsToParents(). The actual placement process starts here: it’s done by: placeSinglePod(), placeDoublePods(), placePodsOnContainer() or placePodsOnScrollableContainer(). The placePods() function decides which one to call based on the number of users we have. There’re lots of details in the placement process but the general logic is simple, because we do not have to determine the best numbers of rows and columns and can just follow the spec instead. One special handling is: when scrollable container is shown it should occupy the full screen, and a gradient mask is applied to avoid blocking the header bar. The z-indexes should be well selected. 2) User removal works as follows: A pod should always be switched to the main pod before being removed (except the trivial 2-user case). Therefore after removing it from its parent (the pod row), the main pod is set to null. We call placePods(), and the following works exactly the same with the 1) scenario. This is because placePods() will determine the main pod and re- append the pods to different parents if necessary. 3) Window resizing works as follows: Once there is a window resizing event, call placePods() directly. The difference with the above two scenarios is that the main pod is not null when window resizing happens. Therefore, placePods() will skip the main pod selection and re-appending children process, and start the actual placement directly. 4) Click event: The only click event on small pods is to trigger the switch between the small pod and the large pod. In the click event handler, we first find out if it’s for a small pod, and if it is, simply switch it with the main pod by switching the parents as well as positions / other relevant styles (handled by changeMainPod()). Please note: a. We do not call placePods() because it may result in reordering the pods. b. It’s OK that the pods are not in LRU order temporarily. This is added to the existing click handler so we do not need to register a new one. BUG=718159 ==========
Description was changed from ========== Multiple user pods implementation for new login screen The new login screen for multiple user pods involve creating small pods and place them on a scroll bar. The ‘small pod’ is implemented as an ‘add-on’ to the existing user pod template, instead of being a new prototype. Pods can easily transform itself among the ‘large’, ‘small’ and ‘extra small’ states, by toggling the visibility of corresponding parts. 1) Pod placement works as follows: Upon receiving the user list from backend, create a new pod for each user (in rebuildPods()), append them to the pow row as children, and call initialize(). This part remains the same with the old implementation, and at this point there is no need to determine what size the pod should be shown. rebuildPods() calls placePods(). placePods() first determines the ‘main pod’ (the one displayed as the large pod), and append all other pods to a ‘small pods container’, because they should be displayed as small pods. This is done in appendPodsToParents(). The actual placement process starts here: it’s done by: placeSinglePod(), placeDoublePods(), placePodsOnContainer() or placePodsOnScrollableContainer(). The placePods() function decides which one to call based on the number of users we have. There’re lots of details in the placement process but the general logic is simple, because we do not have to determine the best numbers of rows and columns and can just follow the spec instead. One special handling is: when scrollable container is shown it should occupy the full screen, and a gradient mask is applied to avoid blocking the header bar. The z-indexes should be well selected. 2) User removal works as follows: A pod should always be switched to the main pod before being removed (except the trivial 2-user case). Therefore after removing it from its parent (the pod row), the main pod is set to null. We call placePods(), and the following works exactly the same with the 1) scenario. This is because placePods() will determine the main pod and re- append the pods to different parents if necessary. 3) Window resizing works as follows: Once there is a window resizing event, call placePods() directly. The difference with the above two scenarios is that the main pod is not null when window resizing happens. Therefore, placePods() will skip the main pod selection and re-appending children process, and start the actual placement directly. 4) Click event: The only click event on small pods is to trigger the switch between the small pod and the large pod. In the click event handler, we first find out if it’s for a small pod, and if it is, simply switch it with the main pod by switching the parents as well as positions / other relevant styles (handled by changeMainPod()). Please note: a. We do not call placePods() because it may result in reordering the pods. b. It’s OK that the pods are not in LRU order temporarily. This is added to the existing click handler so we do not need to register a new one. BUG=718159 ========== to ========== Multiple user pods implementation for new login screen The new login screen for multiple user pods involve creating small pods and place them on a scroll bar. The ‘small pod’ is implemented as an ‘add-on’ to the existing user pod template, instead of being a new prototype. Pods can easily transform itself among the ‘large’, ‘small’ and ‘extra small’ states, by toggling the visibility of corresponding parts. 1) Pod placement works as follows: Upon receiving the user list from backend, create a new pod for each user (in rebuildPods()), append them to the pow row as children, and call initialize(). This part remains the same with the old implementation, and at this point there is no need to determine what size the pod should be shown. rebuildPods() calls placePods(). placePods() first determines the ‘main pod’ (the one displayed as the large pod), and append all other pods to a ‘small pods container’, because they should be displayed as small pods. This is done in appendPodsToParents(). The actual placement process starts here: it’s done by: placeSinglePod_(), placePodsOnPodRow_(), placePodsOnContainer_() or placePodsOnScrollableContainer_(). The placePods() function decides which one to call based on the number of users we have. There’re lots of details in the placement process but the general logic is simple, because we do not have to determine the best numbers of rows and columns and can just follow the spec instead. One special handling is: when scrollable container is shown it should occupy the full screen, and a gradient mask is applied to avoid blocking the header bar. The z-indexes should be well selected. 2) User removal works as follows: A pod should always be switched to the main pod before being removed (except the trivial 2-user case). Therefore after removing it from its parent (the pod row), the main pod is set to null. We call placePods(), and the following works exactly the same with the 1) scenario. This is because placePods() will determine the main pod and re- append the pods to different parents if necessary. 3) Window resizing works as follows: Once there is a window resizing event, call placePods() directly. The difference with the above two scenarios is that the main pod is not null when window resizing happens. Therefore, placePods() will skip the main pod selection and re-appending children process, and start the actual placement directly. 4) Click event: The only click event on small pods is to trigger the switch between the small pod and the large pod. In the click event handler, we first find out if it’s for a small pod, and if it is, simply switch it with the main pod by switching the parents as well as positions / other relevant styles (handled by changeMainPod()). Please note: a. We do not call placePods() because it may result in reordering the pods. b. It’s OK that the pods are not in LRU order temporarily. This is added to the existing click handler so we do not need to register a new one. BUG=718159 ==========
1) Add lots more inline comments. 2) CL description and title have been updated. https://codereview.chromium.org/2898283002/diff/1/ui/login/account_picker/md_... File ui/login/account_picker/md_user_pod_row.css (right): https://codereview.chromium.org/2898283002/diff/1/ui/login/account_picker/md_... ui/login/account_picker/md_user_pod_row.css:1018: .large-pod { On 2017/05/23 21:25:59, rkc wrote: > On 2017/05/23 20:34:32, jdufault wrote: > > As FYI, I've been using the naming convention 'few', 'some', 'many' because it > > avoids the 'small' / 'extra-small' name which is kind of strange. > > > > Though large/small/extra-small is fine. > > These could be defined functionally instead of visually? > For example, we should have a "focused-pod" and "inactive-pod" (just > suggestions) or something along those lines. We should try to stay away from > using terms that are inherently vague like, large, small or even few, some or > many. Discussed offline. We can keep them until we have better names. https://codereview.chromium.org/2898283002/diff/1/ui/login/account_picker/md_... File ui/login/account_picker/md_user_pod_row.js (right): https://codereview.chromium.org/2898283002/diff/1/ui/login/account_picker/md_... ui/login/account_picker/md_user_pod_row.js:1153: this.querySelector('.action-box-remove-user-warning-history'), On 2017/05/23 20:34:32, jdufault wrote: > revert? Reverted. It's changed by style checker. https://codereview.chromium.org/2898283002/diff/1/ui/login/account_picker/md_... ui/login/account_picker/md_user_pod_row.js:1202: this.smallPodImageElement.src = 'chrome://userimage/' + On 2017/05/23 20:34:32, jdufault wrote: > Try to avoid redefining (non-trivial) strings multiple times. > > var imageSrc = ... > this.imageElement.src = imageSrc > this.smallPodImageElement.src = imageSrc Done. https://codereview.chromium.org/2898283002/diff/1/ui/login/account_picker/md_... ui/login/account_picker/md_user_pod_row.js:2378: $('pod-row').changeMainPod_(this); On 2017/05/23 20:34:32, jdufault wrote: > changeMainPod_ seems like it should be a public (ie, no _) function > > Maybe rename to setFocusedPod? Or are you trying to avoid using the name focus? The underscore is removed. I'm open to rename it but I want to avoid 'focus' because the pod occupying the main position and the pod being focused can often be different. https://codereview.chromium.org/2898283002/diff/1/ui/login/account_picker/md_... ui/login/account_picker/md_user_pod_row.js:2383: this.parentNode.setActivatedPod(this, e); On 2017/05/23 20:34:32, jdufault wrote: > If you remove these two calls, does anything break? The two calls must be kept for 'large' pods. They can be removed for 'small' pods, because focusPod() is called by changeMainPod() instead, and setActivatedPod() doesn't apply. https://codereview.chromium.org/2898283002/diff/1/ui/login/account_picker/md_... ui/login/account_picker/md_user_pod_row.js:3473: onWindowResize: function() { On 2017/05/23 20:34:32, jdufault wrote: > Did you test with virtual keyboard? I did. It works, but has bugs related to the background shield. I documented them and will fix them later. scrollFocusedPodIntoView() is removed because it's not applicable to the new design as it shifts the focused pod (when it's at the bottom row) to make it visible. https://codereview.chromium.org/2898283002/diff/1/ui/login/account_picker/md_... ui/login/account_picker/md_user_pod_row.js:3577: for (var i = 0, pod; pod = pods[i]; ++i) { On 2017/05/23 20:34:32, jdufault wrote: > for (var pod of pods)? Done. https://codereview.chromium.org/2898283002/diff/1/ui/login/account_picker/md_... ui/login/account_picker/md_user_pod_row.js:3581: if (pods.length <= 2) { On 2017/05/23 20:34:32, jdufault wrote: > Add comment on the 2 / make it a constant Done. https://codereview.chromium.org/2898283002/diff/1/ui/login/account_picker/md_... ui/login/account_picker/md_user_pod_row.js:3636: if ((pods.length > 6 && !this.isPortraitMode()) || On 2017/05/23 20:34:32, jdufault wrote: > constant for 6 and 10 Done. https://codereview.chromium.org/2898283002/diff/1/ui/login/account_picker/md_... ui/login/account_picker/md_user_pod_row.js:3851: if (pod.isInExtraSmallForm()) { On 2017/05/23 20:34:32, jdufault wrote: > Instead of having separate functions for isExtraSmall, isSmall, it may be easier > to have an enum, ie, > > this.mainPod_.setPodStyle(pod.getPodStyle()) Done.
Description was changed from ========== Multiple user pods implementation for new login screen The new login screen for multiple user pods involve creating small pods and place them on a scroll bar. The ‘small pod’ is implemented as an ‘add-on’ to the existing user pod template, instead of being a new prototype. Pods can easily transform itself among the ‘large’, ‘small’ and ‘extra small’ states, by toggling the visibility of corresponding parts. 1) Pod placement works as follows: Upon receiving the user list from backend, create a new pod for each user (in rebuildPods()), append them to the pow row as children, and call initialize(). This part remains the same with the old implementation, and at this point there is no need to determine what size the pod should be shown. rebuildPods() calls placePods(). placePods() first determines the ‘main pod’ (the one displayed as the large pod), and append all other pods to a ‘small pods container’, because they should be displayed as small pods. This is done in appendPodsToParents(). The actual placement process starts here: it’s done by: placeSinglePod_(), placePodsOnPodRow_(), placePodsOnContainer_() or placePodsOnScrollableContainer_(). The placePods() function decides which one to call based on the number of users we have. There’re lots of details in the placement process but the general logic is simple, because we do not have to determine the best numbers of rows and columns and can just follow the spec instead. One special handling is: when scrollable container is shown it should occupy the full screen, and a gradient mask is applied to avoid blocking the header bar. The z-indexes should be well selected. 2) User removal works as follows: A pod should always be switched to the main pod before being removed (except the trivial 2-user case). Therefore after removing it from its parent (the pod row), the main pod is set to null. We call placePods(), and the following works exactly the same with the 1) scenario. This is because placePods() will determine the main pod and re- append the pods to different parents if necessary. 3) Window resizing works as follows: Once there is a window resizing event, call placePods() directly. The difference with the above two scenarios is that the main pod is not null when window resizing happens. Therefore, placePods() will skip the main pod selection and re-appending children process, and start the actual placement directly. 4) Click event: The only click event on small pods is to trigger the switch between the small pod and the large pod. In the click event handler, we first find out if it’s for a small pod, and if it is, simply switch it with the main pod by switching the parents as well as positions / other relevant styles (handled by changeMainPod()). Please note: a. We do not call placePods() because it may result in reordering the pods. b. It’s OK that the pods are not in LRU order temporarily. This is added to the existing click handler so we do not need to register a new one. BUG=718159 ========== to ========== Multiple user pods implementation for new login screen The new login screen for multiple user pods involve creating small pods and place them on a scroll bar. The ‘small pod’ is implemented as an ‘add-on’ to the existing user pod template, instead of being a new prototype. Pods can easily transform itself among the ‘large’, ‘small’ and ‘extra small’ states, by toggling the visibility of corresponding parts. 1) Pod placement works as follows: Upon receiving the user list from backend, create a new pod for each user (in rebuildPods()), append them to the pow row as children, and call initialize(). This part remains the same with the old implementation, and at this point there is no need to determine what size the pod should be shown. rebuildPods() calls placePods(). placePods() first determines the ‘main pod’ (the one displayed as the large pod), and append all other pods to a ‘small pods container’, because they should be displayed as small pods. This is done in appendPodsToParents(). The actual placement process starts here: it’s done by: placeSinglePod_(), placePodsOnPodRow_(), placePodsOnContainer_() or placePodsOnScrollableContainer_(). The placePods() function decides which one to call based on the number of users we have. There’re lots of details in the placement process but the general logic is simple, because we do not have to determine the best numbers of rows and columns and can just follow the spec instead. One special handling is: when scrollable container is shown it should occupy the full screen, and a gradient mask is applied to avoid blocking the header bar. The z-indexes should be well selected. 2) User removal works as follows: A pod should always be switched to the main pod before being removed (except the trivial 2-user case). Therefore after removing it from its parent (the pod row), the main pod is set to null. We call placePods(), and the following works exactly the same with the 1) scenario. This is because placePods() will determine the main pod and re- append the pods to different parents if necessary. 3) Window resizing works as follows: Once there is a window resizing event, call placePods() directly. The difference with the above two scenarios is that the main pod is not null when window resizing happens. Therefore, placePods() will skip the main pod selection and re-appending children process, and start the actual placement directly. 4) Click event: The only click event on small pods is to trigger the switch between the small pod and the large pod. In the click event handler, we first find out if it’s for a small pod, and if it is, simply switch it with the main pod by switching the parents as well as positions / other relevant styles (handled by changeMainPod()). Please note: We do not call placePods() because it may result in reordering the pods. It’s OK that the pods are not in LRU order temporarily. a new one. BUG=718159 ==========
Description was changed from ========== Multiple user pods implementation for new login screen The new login screen for multiple user pods involve creating small pods and place them on a scroll bar. The ‘small pod’ is implemented as an ‘add-on’ to the existing user pod template, instead of being a new prototype. Pods can easily transform itself among the ‘large’, ‘small’ and ‘extra small’ states, by toggling the visibility of corresponding parts. 1) Pod placement works as follows: Upon receiving the user list from backend, create a new pod for each user (in rebuildPods()), append them to the pow row as children, and call initialize(). This part remains the same with the old implementation, and at this point there is no need to determine what size the pod should be shown. rebuildPods() calls placePods(). placePods() first determines the ‘main pod’ (the one displayed as the large pod), and append all other pods to a ‘small pods container’, because they should be displayed as small pods. This is done in appendPodsToParents(). The actual placement process starts here: it’s done by: placeSinglePod_(), placePodsOnPodRow_(), placePodsOnContainer_() or placePodsOnScrollableContainer_(). The placePods() function decides which one to call based on the number of users we have. There’re lots of details in the placement process but the general logic is simple, because we do not have to determine the best numbers of rows and columns and can just follow the spec instead. One special handling is: when scrollable container is shown it should occupy the full screen, and a gradient mask is applied to avoid blocking the header bar. The z-indexes should be well selected. 2) User removal works as follows: A pod should always be switched to the main pod before being removed (except the trivial 2-user case). Therefore after removing it from its parent (the pod row), the main pod is set to null. We call placePods(), and the following works exactly the same with the 1) scenario. This is because placePods() will determine the main pod and re- append the pods to different parents if necessary. 3) Window resizing works as follows: Once there is a window resizing event, call placePods() directly. The difference with the above two scenarios is that the main pod is not null when window resizing happens. Therefore, placePods() will skip the main pod selection and re-appending children process, and start the actual placement directly. 4) Click event: The only click event on small pods is to trigger the switch between the small pod and the large pod. In the click event handler, we first find out if it’s for a small pod, and if it is, simply switch it with the main pod by switching the parents as well as positions / other relevant styles (handled by changeMainPod()). Please note: We do not call placePods() because it may result in reordering the pods. It’s OK that the pods are not in LRU order temporarily. a new one. BUG=718159 ========== to ========== Multiple user pods implementation for new login screen The new login screen for multiple user pods involve creating small pods and place them on a scroll bar. The ‘small pod’ is implemented as an ‘add-on’ to the existing user pod template, instead of being a new prototype. Pods can easily transform itself among the ‘large’, ‘small’ and ‘extra small’ states, by toggling the visibility of corresponding parts. 1) Pod placement works as follows: Upon receiving the user list from backend, create a new pod for each user (in rebuildPods()), append them to the pow row as children, and call initialize(). This part remains the same with the old implementation, and at this point there is no need to determine what size the pod should be shown. rebuildPods() calls placePods(). placePods() first determines the ‘main pod’ (the one displayed as the large pod), and append all other pods to a ‘small pods container’, because they should be displayed as small pods. This is done in appendPodsToParents(). The actual placement process starts here: it’s done by: placeSinglePod_(), placePodsOnPodRow_(), placePodsOnContainer_() or placePodsOnScrollableContainer_(). The placePods() function decides which one to call based on the number of users we have. There’re lots of details in the placement process but the general logic is simple, because we do not have to determine the best numbers of rows and columns and can just follow the spec instead. One special handling is: when scrollable container is shown it should occupy the full screen, and a gradient mask is applied to avoid blocking the header bar. The z-indexes should be well selected. 2) User removal works as follows: A pod should always be switched to the main pod before being removed (except the trivial 2-user case). Therefore after removing it from its parent (the pod row), the main pod is set to null. We call placePods(), and the following works exactly the same with the 1) scenario. This is because placePods() will determine the main pod and re- append the pods to different parents if necessary. 3) Window resizing works as follows: Once there is a window resizing event, call placePods() directly. The difference with the above two scenarios is that the main pod is not null when window resizing happens. Therefore, placePods() will skip the main pod selection and re-appending children process, and start the actual placement directly. 4) Click event: The only click event on small pods is to trigger the switch between the small pod and the large pod. In the click event handler, we first find out if it’s for a small pod, and if it is, simply switch it with the main pod by switching the parents as well as positions / other relevant styles (handled by changeMainPod()). Please note: We do not call placePods() because it may result in reordering the pods. It’s OK that the pods are not in LRU order temporarily. BUG=718159 ==========
https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... File ui/login/account_picker/md_user_pod_row.js (right): https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:1191: * @type {number} This doesn't return anything so there shouldn't be a @type param? @param {UserPod.Style} style https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:1193: set podStyle(style) { FYI: In the future I recommend making any non-trivial getter/setter a function call, so it is clear to the reader it may be doing a significant amount of work (but I don't think we have anything codified in the style guide around this). Feel free to leave this code as-is. https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:1217: * @type {number} @type {UserPod.Style} https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:2075: // Small pods do not have mouse down event. The comment is a little confusing - I'd recommend negating it because the if deals only with UserPod.Style.LARGE // Only large pods have mouse down events https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:2170: this.handleSmallPodKeyDown_(e); handleNonLargePodKeyDown? https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:2694: // It should trigger the switch between the small pod and the main pod. This comment is a restatement of relatively obvious code. It's probably not worth keeping - but if you feel differently feel free to leave it. https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:2785: $('pod-row').changeMainPod(this); FYI: 'change' is an unusual word, typically 'set' is used instead (it is shorter). In this 'switchMainPod' or 'swapMainPod' is most clear. https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:3509: this.placePods_(); Does this happen on orientation change? https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:3725: if (pod == this.mainPod_) { FYI: style guide says to drop the {} on single-line ifs. We're more consistent about it on C++ than JS though. https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:3835: // reset all of them to be safe. Any idea why? It'd be good to include that in the comment if you know. https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:3841: * Check if it's in portrait mode. Please clarify "it" https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:3911: if (pod.podStyle == UserPod.Style.EXTRA_SMALL) { this.mainPod_.podStyle = pod.podStyle? Then you can eliminate this if/else block.
https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... File ui/login/account_picker/md_user_pod_row.js (right): https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:1191: * @type {number} This doesn't return anything so there shouldn't be a @type param? @param {UserPod.Style} style https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:1193: set podStyle(style) { FYI: In the future I recommend making any non-trivial getter/setter a function call, so it is clear to the reader it may be doing a significant amount of work (but I don't think we have anything codified in the style guide around this). Feel free to leave this code as-is. https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:2075: // Small pods do not have mouse down event. The comment is a little confusing - I'd recommend negating it because the if deals only with UserPod.Style.LARGE // Only large pods have mouse down events https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:2694: // It should trigger the switch between the small pod and the main pod. This comment is a restatement of relatively obvious code. It's probably not worth keeping - but if you feel differently feel free to leave it. https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:2785: $('pod-row').changeMainPod(this); FYI: 'change' is an unusual word, typically 'set' is used instead (it is shorter). In this 'switchMainPod' or 'swapMainPod' is most clear. https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:3835: // reset all of them to be safe. Any idea why? It'd be good to include that in the comment if you know. https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:3841: * Check if it's in portrait mode. Please clarify "it"
jdufault@, In onWindowResize I added a fix to an existing problem. PTAL. Thanks so much. https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... File ui/login/account_picker/md_user_pod_row.js (right): https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:1191: * @type {number} On 2017/05/24 17:11:15, jdufault wrote: > This doesn't return anything so there shouldn't be a @type param? > > @param {UserPod.Style} style Done. https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:1193: set podStyle(style) { On 2017/05/24 17:11:15, jdufault wrote: > FYI: In the future I recommend making any non-trivial getter/setter a function > call, so it is clear to the reader it may be doing a significant amount of work > (but I don't think we have anything codified in the style guide around this). > Feel free to leave this code as-is. Done. https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:1217: * @type {number} On 2017/05/24 17:11:14, jdufault wrote: > @type {UserPod.Style} Done. https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:2170: this.handleSmallPodKeyDown_(e); On 2017/05/24 17:11:14, jdufault wrote: > handleNonLargePodKeyDown? Done. https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:2694: // It should trigger the switch between the small pod and the main pod. On 2017/05/24 17:11:15, jdufault wrote: > This comment is a restatement of relatively obvious code. It's probably not > worth keeping - but if you feel differently feel free to leave it. Acknowledged. https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:2785: $('pod-row').changeMainPod(this); On 2017/05/24 17:11:15, jdufault wrote: > FYI: 'change' is an unusual word, typically 'set' is used instead (it is > shorter). In this 'switchMainPod' or 'swapMainPod' is most clear. Done. https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:3509: this.placePods_(); On 2017/05/24 17:11:14, jdufault wrote: > Does this happen on orientation change? Yes. onWindowResize is called when changing the screen orientation or showing the virtual keyboard. I need to figure out all other use cases. In addition, I added the codes to handle a corner case for virtual keyboard: When users scroll upward they can see the empty space between the account picker and the virtual keyboard, which is quite ugly. I'm afraid it's not a very good solution but works fine after trial and error. https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:3725: if (pod == this.mainPod_) { On 2017/05/24 17:11:14, jdufault wrote: > FYI: style guide says to drop the {} on single-line ifs. We're more consistent > about it on C++ than JS though. I will pay attention to it. https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:3835: // reset all of them to be safe. On 2017/05/24 17:11:15, jdufault wrote: > Any idea why? It'd be good to include that in the comment if you know. Done. https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:3841: * Check if it's in portrait mode. On 2017/05/24 17:11:15, jdufault wrote: > Please clarify "it" Done. https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:3911: if (pod.podStyle == UserPod.Style.EXTRA_SMALL) { On 2017/05/24 17:11:14, jdufault wrote: > this.mainPod_.podStyle = pod.podStyle? > > Then you can eliminate this if/else block. Done.
https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... File ui/login/account_picker/md_user_pod_row.js (right): https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:1191: * @type {number} On 2017/05/24 19:23:43, Wenzhao (Colin) Zang wrote: > On 2017/05/24 17:11:15, jdufault wrote: > > This doesn't return anything so there shouldn't be a @type param? > > > > @param {UserPod.Style} style > > Done. Needs @param type annotation https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:1193: set podStyle(style) { On 2017/05/24 19:23:43, Wenzhao (Colin) Zang wrote: > On 2017/05/24 17:11:15, jdufault wrote: > > FYI: In the future I recommend making any non-trivial getter/setter a function > > call, so it is clear to the reader it may be doing a significant amount of > work > > (but I don't think we have anything codified in the style guide around this). > > Feel free to leave this code as-is. > > Done. The idea here is to have it clear at the call site this is non-trivial. So instead of having `pod.podStyle = Foo`, you have `pod.setPodStyle(Foo)` at the call site. https://codereview.chromium.org/2898283002/diff/60001/ui/login/account_picker... File ui/login/account_picker/md_user_pod_row.js (right): https://codereview.chromium.org/2898283002/diff/60001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:3530: // is reduced properly, the size of the outer container remains the Nice comment https://codereview.chromium.org/2898283002/diff/60001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:3942: console.error('Cannot change main pod while sign-in UI is disabled.'); I wonder if the entire sign-in screen is disabled, or just the active user? If disabled for just the active user, we should probably allow switching.
https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... File ui/login/account_picker/md_user_pod_row.js (right): https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:1191: * @type {number} On 2017/05/24 20:44:52, jdufault wrote: > On 2017/05/24 19:23:43, Wenzhao (Colin) Zang wrote: > > On 2017/05/24 17:11:15, jdufault wrote: > > > This doesn't return anything so there shouldn't be a @type param? > > > > > > @param {UserPod.Style} style > > > > Done. > > Needs @param type annotation Done. https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:1193: set podStyle(style) { On 2017/05/24 20:44:52, jdufault wrote: > On 2017/05/24 19:23:43, Wenzhao (Colin) Zang wrote: > > On 2017/05/24 17:11:15, jdufault wrote: > > > FYI: In the future I recommend making any non-trivial getter/setter a > function > > > call, so it is clear to the reader it may be doing a significant amount of > > work > > > (but I don't think we have anything codified in the style guide around > this). > > > Feel free to leave this code as-is. > > > > Done. > > The idea here is to have it clear at the call site this is non-trivial. So > instead of having `pod.podStyle = Foo`, you have `pod.setPodStyle(Foo)` at the > call site. Oh I see. I misunderstood earlier. https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:1193: set podStyle(style) { On 2017/05/24 20:44:52, jdufault wrote: > On 2017/05/24 19:23:43, Wenzhao (Colin) Zang wrote: > > On 2017/05/24 17:11:15, jdufault wrote: > > > FYI: In the future I recommend making any non-trivial getter/setter a > function > > > call, so it is clear to the reader it may be doing a significant amount of > > work > > > (but I don't think we have anything codified in the style guide around > this). > > > Feel free to leave this code as-is. > > > > Done. > > The idea here is to have it clear at the call site this is non-trivial. So > instead of having `pod.podStyle = Foo`, you have `pod.setPodStyle(Foo)` at the > call site. Done. https://codereview.chromium.org/2898283002/diff/40001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:2694: // It should trigger the switch between the small pod and the main pod. On 2017/05/24 19:23:43, Wenzhao (Colin) Zang wrote: > On 2017/05/24 17:11:15, jdufault wrote: > > This comment is a restatement of relatively obvious code. It's probably not > > worth keeping - but if you feel differently feel free to leave it. > > Acknowledged. Done. https://codereview.chromium.org/2898283002/diff/60001/ui/login/account_picker... File ui/login/account_picker/md_user_pod_row.js (right): https://codereview.chromium.org/2898283002/diff/60001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:3530: // is reduced properly, the size of the outer container remains the On 2017/05/24 20:44:52, jdufault wrote: > Nice comment :) https://codereview.chromium.org/2898283002/diff/60001/ui/login/account_picker... ui/login/account_picker/md_user_pod_row.js:3942: console.error('Cannot change main pod while sign-in UI is disabled.'); On 2017/05/24 20:44:52, jdufault wrote: > I wonder if the entire sign-in screen is disabled, or just the active user? If > disabled for just the active user, we should probably allow switching. This is for the pow row. Please see lines 2950 - 2960.
lgtm
alemate@: please take a look at your earliest convenience. We hope to start testing soon. Thanks!
lgtm
The CQ bit was checked by wzang@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": 80001, "attempt_start_ts": 1495763731922840,
"parent_rev": "37b5829be1daf71af6e35354c2f39a4eec37c60d", "commit_rev":
"c1604bc83510da9806cb1c00d3351d8fcb7eb24a"}
Message was sent while issue was closed.
Description was changed from ========== Multiple user pods implementation for new login screen The new login screen for multiple user pods involve creating small pods and place them on a scroll bar. The ‘small pod’ is implemented as an ‘add-on’ to the existing user pod template, instead of being a new prototype. Pods can easily transform itself among the ‘large’, ‘small’ and ‘extra small’ states, by toggling the visibility of corresponding parts. 1) Pod placement works as follows: Upon receiving the user list from backend, create a new pod for each user (in rebuildPods()), append them to the pow row as children, and call initialize(). This part remains the same with the old implementation, and at this point there is no need to determine what size the pod should be shown. rebuildPods() calls placePods(). placePods() first determines the ‘main pod’ (the one displayed as the large pod), and append all other pods to a ‘small pods container’, because they should be displayed as small pods. This is done in appendPodsToParents(). The actual placement process starts here: it’s done by: placeSinglePod_(), placePodsOnPodRow_(), placePodsOnContainer_() or placePodsOnScrollableContainer_(). The placePods() function decides which one to call based on the number of users we have. There’re lots of details in the placement process but the general logic is simple, because we do not have to determine the best numbers of rows and columns and can just follow the spec instead. One special handling is: when scrollable container is shown it should occupy the full screen, and a gradient mask is applied to avoid blocking the header bar. The z-indexes should be well selected. 2) User removal works as follows: A pod should always be switched to the main pod before being removed (except the trivial 2-user case). Therefore after removing it from its parent (the pod row), the main pod is set to null. We call placePods(), and the following works exactly the same with the 1) scenario. This is because placePods() will determine the main pod and re- append the pods to different parents if necessary. 3) Window resizing works as follows: Once there is a window resizing event, call placePods() directly. The difference with the above two scenarios is that the main pod is not null when window resizing happens. Therefore, placePods() will skip the main pod selection and re-appending children process, and start the actual placement directly. 4) Click event: The only click event on small pods is to trigger the switch between the small pod and the large pod. In the click event handler, we first find out if it’s for a small pod, and if it is, simply switch it with the main pod by switching the parents as well as positions / other relevant styles (handled by changeMainPod()). Please note: We do not call placePods() because it may result in reordering the pods. It’s OK that the pods are not in LRU order temporarily. BUG=718159 ========== to ========== Multiple user pods implementation for new login screen The new login screen for multiple user pods involve creating small pods and place them on a scroll bar. The ‘small pod’ is implemented as an ‘add-on’ to the existing user pod template, instead of being a new prototype. Pods can easily transform itself among the ‘large’, ‘small’ and ‘extra small’ states, by toggling the visibility of corresponding parts. 1) Pod placement works as follows: Upon receiving the user list from backend, create a new pod for each user (in rebuildPods()), append them to the pow row as children, and call initialize(). This part remains the same with the old implementation, and at this point there is no need to determine what size the pod should be shown. rebuildPods() calls placePods(). placePods() first determines the ‘main pod’ (the one displayed as the large pod), and append all other pods to a ‘small pods container’, because they should be displayed as small pods. This is done in appendPodsToParents(). The actual placement process starts here: it’s done by: placeSinglePod_(), placePodsOnPodRow_(), placePodsOnContainer_() or placePodsOnScrollableContainer_(). The placePods() function decides which one to call based on the number of users we have. There’re lots of details in the placement process but the general logic is simple, because we do not have to determine the best numbers of rows and columns and can just follow the spec instead. One special handling is: when scrollable container is shown it should occupy the full screen, and a gradient mask is applied to avoid blocking the header bar. The z-indexes should be well selected. 2) User removal works as follows: A pod should always be switched to the main pod before being removed (except the trivial 2-user case). Therefore after removing it from its parent (the pod row), the main pod is set to null. We call placePods(), and the following works exactly the same with the 1) scenario. This is because placePods() will determine the main pod and re- append the pods to different parents if necessary. 3) Window resizing works as follows: Once there is a window resizing event, call placePods() directly. The difference with the above two scenarios is that the main pod is not null when window resizing happens. Therefore, placePods() will skip the main pod selection and re-appending children process, and start the actual placement directly. 4) Click event: The only click event on small pods is to trigger the switch between the small pod and the large pod. In the click event handler, we first find out if it’s for a small pod, and if it is, simply switch it with the main pod by switching the parents as well as positions / other relevant styles (handled by changeMainPod()). Please note: We do not call placePods() because it may result in reordering the pods. It’s OK that the pods are not in LRU order temporarily. BUG=718159 Review-Url: https://codereview.chromium.org/2898283002 Cr-Commit-Position: refs/heads/master@{#474889} Committed: https://chromium.googlesource.com/chromium/src/+/c1604bc83510da9806cb1c00d335... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/src/+/c1604bc83510da9806cb1c00d335...
Message was sent while issue was closed.
Description was changed from ========== Multiple user pods implementation for new login screen The new login screen for multiple user pods involve creating small pods and place them on a scroll bar. The ‘small pod’ is implemented as an ‘add-on’ to the existing user pod template, instead of being a new prototype. Pods can easily transform itself among the ‘large’, ‘small’ and ‘extra small’ states, by toggling the visibility of corresponding parts. 1) Pod placement works as follows: Upon receiving the user list from backend, create a new pod for each user (in rebuildPods()), append them to the pow row as children, and call initialize(). This part remains the same with the old implementation, and at this point there is no need to determine what size the pod should be shown. rebuildPods() calls placePods(). placePods() first determines the ‘main pod’ (the one displayed as the large pod), and append all other pods to a ‘small pods container’, because they should be displayed as small pods. This is done in appendPodsToParents(). The actual placement process starts here: it’s done by: placeSinglePod_(), placePodsOnPodRow_(), placePodsOnContainer_() or placePodsOnScrollableContainer_(). The placePods() function decides which one to call based on the number of users we have. There’re lots of details in the placement process but the general logic is simple, because we do not have to determine the best numbers of rows and columns and can just follow the spec instead. One special handling is: when scrollable container is shown it should occupy the full screen, and a gradient mask is applied to avoid blocking the header bar. The z-indexes should be well selected. 2) User removal works as follows: A pod should always be switched to the main pod before being removed (except the trivial 2-user case). Therefore after removing it from its parent (the pod row), the main pod is set to null. We call placePods(), and the following works exactly the same with the 1) scenario. This is because placePods() will determine the main pod and re- append the pods to different parents if necessary. 3) Window resizing works as follows: Once there is a window resizing event, call placePods() directly. The difference with the above two scenarios is that the main pod is not null when window resizing happens. Therefore, placePods() will skip the main pod selection and re-appending children process, and start the actual placement directly. 4) Click event: The only click event on small pods is to trigger the switch between the small pod and the large pod. In the click event handler, we first find out if it’s for a small pod, and if it is, simply switch it with the main pod by switching the parents as well as positions / other relevant styles (handled by changeMainPod()). Please note: We do not call placePods() because it may result in reordering the pods. It’s OK that the pods are not in LRU order temporarily. BUG=718159 Review-Url: https://codereview.chromium.org/2898283002 Cr-Commit-Position: refs/heads/master@{#474889} Committed: https://chromium.googlesource.com/chromium/src/+/c1604bc83510da9806cb1c00d335... ========== to ========== Multiple user pods implementation for new login screen The new login screen for multiple user pods involves creating small pods and places them on a scroll bar. The ‘small pod’ is implemented as an ‘add-on’ to the existing user pod template, instead of being a new prototype. Pods can easily transform itself among the ‘large’, ‘small’ and ‘extra small’ states, by toggling the visibility of corresponding parts. 1) Pod placement works as follows: Upon receiving the user list from backend, create a new pod for each user (in rebuildPods()), append them to the pow row as children, and call initialize(). This part remains the same with the old implementation, and at this point there is no need to determine what size the pod should be shown. rebuildPods() calls placePods(). placePods() first determines the ‘main pod’ (the one displayed as the large pod), and append all other pods to a ‘small pods container’, because they should be displayed as small pods. This is done in appendPodsToParents(). The actual placement process starts here: it’s done by: placeSinglePod_(), placePodsOnPodRow_(), placePodsOnContainer_() or placePodsOnScrollableContainer_(). The placePods() function decides which one to call based on the number of users we have. There’re lots of details in the placement process but the general logic is simple, because we do not have to determine the best numbers of rows and columns and can just follow the spec instead. One special handling is: when scrollable container is shown it should occupy the full screen, and a gradient mask is applied to avoid blocking the header bar. The z-indexes should be well selected. 2) User removal works as follows: A pod should always be switched to the main pod before being removed (except the trivial 2-user case). Therefore after removing it from its parent (the pod row), the main pod is set to null. We call placePods(), and the following works exactly the same with the 1) scenario. This is because placePods() will determine the main pod and re- append the pods to different parents if necessary. 3) Window resizing works as follows: Once there is a window resizing event, call placePods() directly. The difference with the above two scenarios is that the main pod is not null when window resizing happens. Therefore, placePods() will skip the main pod selection and re-appending children process, and start the actual placement directly. 4) Click event: The only click event on small pods is to trigger the switch between the small pod and the large pod. In the click event handler, we first find out if it’s for a small pod, and if it is, simply switch it with the main pod by switching the parents as well as positions / other relevant styles (handled by changeMainPod()). Please note: We do not call placePods() because it may result in reordering the pods. It’s OK that the pods are not in LRU order temporarily. BUG=718159 Review-Url: https://codereview.chromium.org/2898283002 Cr-Commit-Position: refs/heads/master@{#474889} Committed: https://chromium.googlesource.com/chromium/src/+/c1604bc83510da9806cb1c00d335... ==========
Message was sent while issue was closed.
Description was changed from ========== Multiple user pods implementation for new login screen The new login screen for multiple user pods involves creating small pods and places them on a scroll bar. The ‘small pod’ is implemented as an ‘add-on’ to the existing user pod template, instead of being a new prototype. Pods can easily transform itself among the ‘large’, ‘small’ and ‘extra small’ states, by toggling the visibility of corresponding parts. 1) Pod placement works as follows: Upon receiving the user list from backend, create a new pod for each user (in rebuildPods()), append them to the pow row as children, and call initialize(). This part remains the same with the old implementation, and at this point there is no need to determine what size the pod should be shown. rebuildPods() calls placePods(). placePods() first determines the ‘main pod’ (the one displayed as the large pod), and append all other pods to a ‘small pods container’, because they should be displayed as small pods. This is done in appendPodsToParents(). The actual placement process starts here: it’s done by: placeSinglePod_(), placePodsOnPodRow_(), placePodsOnContainer_() or placePodsOnScrollableContainer_(). The placePods() function decides which one to call based on the number of users we have. There’re lots of details in the placement process but the general logic is simple, because we do not have to determine the best numbers of rows and columns and can just follow the spec instead. One special handling is: when scrollable container is shown it should occupy the full screen, and a gradient mask is applied to avoid blocking the header bar. The z-indexes should be well selected. 2) User removal works as follows: A pod should always be switched to the main pod before being removed (except the trivial 2-user case). Therefore after removing it from its parent (the pod row), the main pod is set to null. We call placePods(), and the following works exactly the same with the 1) scenario. This is because placePods() will determine the main pod and re- append the pods to different parents if necessary. 3) Window resizing works as follows: Once there is a window resizing event, call placePods() directly. The difference with the above two scenarios is that the main pod is not null when window resizing happens. Therefore, placePods() will skip the main pod selection and re-appending children process, and start the actual placement directly. 4) Click event: The only click event on small pods is to trigger the switch between the small pod and the large pod. In the click event handler, we first find out if it’s for a small pod, and if it is, simply switch it with the main pod by switching the parents as well as positions / other relevant styles (handled by changeMainPod()). Please note: We do not call placePods() because it may result in reordering the pods. It’s OK that the pods are not in LRU order temporarily. BUG=718159 Review-Url: https://codereview.chromium.org/2898283002 Cr-Commit-Position: refs/heads/master@{#474889} Committed: https://chromium.googlesource.com/chromium/src/+/c1604bc83510da9806cb1c00d335... ========== to ========== Multiple user pods implementation for new login screen The new login screen for multiple user pods involves creating small pods and places them on a scroll bar. The ‘small pod’ is implemented as an ‘add-on’ to the existing user pod template, instead of being a new prototype. Pods can easily transform itself among the ‘large’, ‘small’ and ‘extra small’ states, by toggling the visibility of corresponding parts. 1) Pod placement works as follows: Upon receiving the user list from backend, create a new pod for each user (in rebuildPods()), append them to the pow row as children, and call initialize(). This part remains the same with the old implementation, and at this point there is no need to determine what size the pod should be shown. rebuildPods() calls placePods(). placePods() first determines the ‘main pod’ (the one displayed as the large pod), and append all other pods to a ‘small pods container’, because they should be displayed as small pods. This is done in appendPodsToParents(). The actual placement process starts here: it’s done by: placeSinglePod_(), placePodsOnPodRow_(), placePodsOnContainer_() or placePodsOnScrollableContainer_(). The placePods() function decides which one to call based on the number of users we have. There’re lots of details in the placement process but the general logic is simple, because we do not have to determine the best numbers of rows and columns and can just follow the spec instead. One special handling is: when scrollable container is shown it should occupy the full screen, and a gradient mask is applied to avoid blocking the header bar. The z-indexes should be well selected. 2) User removal works as follows: A pod should always be switched to the main pod before being removed (except the trivial 2-user case). Therefore after removing it from its parent (the pod row), the main pod is set to null. We call placePods(), and the following works exactly the same with the 1) scenario. This is because placePods() will determine the main pod and re- append the pods to different parents if necessary. 3) Window resizing works as follows: Once there is a window resizing event, call placePods() directly. The difference with the above two scenarios is that the main pod is not null when window resizing happens. Therefore, placePods() will skip the main pod selection and re-appending children process, and start the actual placement directly. 4) Click event: The only click event on small pods is to trigger the switch between the small pod and the large pod. In the click event handler, we first find out if it’s for a small pod, and if it is, simply switch it with the main pod by switching the parents as well as positions / other relevant styles (handled by changeMainPod()). Please note: We do not call placePods() because it may result in reordering the pods. It’s OK that the pods are not in LRU order temporarily. BUG=718159 Review-Url: https://codereview.chromium.org/2898283002 Cr-Commit-Position: refs/heads/master@{#474889} Committed: https://chromium.googlesource.com/chromium/src/+/c1604bc83510da9806cb1c00d335... ========== |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
