New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 884058 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 1
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Feature

Blocking:
issue 870128
issue 883917
issue 884065
issue 889574


Participants' hotlists:
Better-Together-Launch-Blockers


Sign in to add a comment

Refactor MultiDeviceSetup button functionality

Project Member Reported by khorimoto@chromium.org, Sep 14

Issue description

In its current design, we've implemented our own buttons (see [1]). However, OOBE buttons are styled differently and have their own custom Polymer elements (see [2]).

We need to refactor how buttons are rendered to support both modes.

[1] https://cs.chromium.org/chromium/src/chrome/browser/resources/chromeos/multidevice_setup/button_bar.html
[2] https://cs.chromium.org/chromium/src/chrome/browser/resources/chromeos/login/oobe_buttons.html
 
Blocking: 884065
Status: Started (was: Assigned)
Components: -UI>ProximityAuth UI>Multidevice
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 25

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

commit fdeeb64b8d433762ac723e8b1021adabc606233e
Author: Kyle Horimoto <khorimoto@google.com>
Date: Tue Sep 25 00:43:24 2018

[CrOS MultiDevice] Use hybrid @apply syntax.

This CL changes "@apply(--foo);" to "@apply --foo;" for all affected
MultiDevice CSS. See [1] and [2] for context.

[1] https://www.polymer-project.org/2.0/docs/upgrade#shadow-dom-styles
[2] https://bugs.chromium.org/p/chromium/issues/detail?id=739532

Bug:  884058 
Change-Id: Id7a0c3e479cae58bfea736093883c92ae44f1d61
Reviewed-on: https://chromium-review.googlesource.com/1242055
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593781}
[modify] https://crrev.com/fdeeb64b8d433762ac723e8b1021adabc606233e/chrome/browser/resources/settings/multidevice_page/multidevice_subpage.html
[modify] https://crrev.com/fdeeb64b8d433762ac723e8b1021adabc606233e/ui/webui/resources/cr_components/chromeos/multidevice_setup/button_bar.html
[modify] https://crrev.com/fdeeb64b8d433762ac723e8b1021adabc606233e/ui/webui/resources/cr_components/chromeos/multidevice_setup/multidevice_setup.html
[modify] https://crrev.com/fdeeb64b8d433762ac723e8b1021adabc606233e/ui/webui/resources/cr_components/chromeos/multidevice_setup/password_page.html
[modify] https://crrev.com/fdeeb64b8d433762ac723e8b1021adabc606233e/ui/webui/resources/cr_components/chromeos/multidevice_setup/setup_succeeded_page.html
[modify] https://crrev.com/fdeeb64b8d433762ac723e8b1021adabc606233e/ui/webui/resources/cr_components/chromeos/multidevice_setup/start_setup_page.html

Blocking: 889574
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 26

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

commit d34074f0ba4e631a2bb43681538677c34318550d
Author: Kyle Horimoto <khorimoto@google.com>
Date: Wed Sep 26 23:23:11 2018

[CrOS MultiDevice] Convert MojoApiBehavior to class-based approach.

The previous approach used a behavior, meaning that any Polymer element
which needs to access the JS Mojo API needs to create its own
InterfacePtr to the Mojo service. In a follow-up CL, I will be adding a
new client of this interface, so it would have been wasteful to create
two separate connections to the service.

This CL converts this behavior to a class-based approach which utilizes
a singleton class which stores a single instance of the pointer.

Bug:  884058 
Change-Id: I0b2c13677309b9949a0f26d2f11dd40ea8f15b1c
Reviewed-on: https://chromium-review.googlesource.com/1247035
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594518}
[modify] https://crrev.com/d34074f0ba4e631a2bb43681538677c34318550d/chrome/test/data/webui/multidevice_setup/integration_test.js
[modify] https://crrev.com/d34074f0ba4e631a2bb43681538677c34318550d/ui/webui/resources/cr_components/chromeos/multidevice_setup/BUILD.gn
[rename] https://crrev.com/d34074f0ba4e631a2bb43681538677c34318550d/ui/webui/resources/cr_components/chromeos/multidevice_setup/mojo_api.html
[add] https://crrev.com/d34074f0ba4e631a2bb43681538677c34318550d/ui/webui/resources/cr_components/chromeos/multidevice_setup/mojo_api.js
[delete] https://crrev.com/5434171553ff93ccc73d236005189dfebd2a5d04/ui/webui/resources/cr_components/chromeos/multidevice_setup/mojo_api_behavior.js
[modify] https://crrev.com/d34074f0ba4e631a2bb43681538677c34318550d/ui/webui/resources/cr_components/chromeos/multidevice_setup/multidevice_setup.html
[modify] https://crrev.com/d34074f0ba4e631a2bb43681538677c34318550d/ui/webui/resources/cr_components/chromeos/multidevice_setup/multidevice_setup.js
[modify] https://crrev.com/d34074f0ba4e631a2bb43681538677c34318550d/ui/webui/resources/cr_components/cr_components_resources.grdp

Project Member

Comment 7 by bugdroid1@chromium.org, Sep 28

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

commit caba515ed1c02bff785651734da65f6ee1516e42
Author: Kyle Horimoto <khorimoto@google.com>
Date: Fri Sep 28 23:53:05 2018

[CrOS MultiDevice] Remove UiMode in favor of MultiDeviceSetupDelegate.

Before this CL, the <multidevice-setup> element was told what "UI mode"
(i.e., OOBE vs. post-OOBE) and behaved differently depending on what
mode was active. This abstraction was leaky, since it caused the element
to change its logic based on what its client at a higher level was.

This CL adds a MultiDeviceSetupDelegate interface and injects this
interface into the <multidevice-setup> element, which uses it to perform
tasks such as setting the host device. One concrete implementation of
this interface is provided for the post-OOBE case; a future CL will add
an implementation for OOBE.

This CL also removes multidevice_setup_dialog.js in favor of adding a
new <multidevice-setup-post-oobe> element.

Bug:  884058 
Change-Id: I76b6553928e706da4870339d306bc32bea7ac3e6
Reviewed-on: https://chromium-review.googlesource.com/1247525
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595258}
[modify] https://crrev.com/caba515ed1c02bff785651734da65f6ee1516e42/chrome/browser/resources/chromeos/multidevice_setup/BUILD.gn
[modify] https://crrev.com/caba515ed1c02bff785651734da65f6ee1516e42/chrome/browser/resources/chromeos/multidevice_setup/multidevice_setup_dialog.html
[delete] https://crrev.com/ef5f45aebcc6b09dca18c841d43c85498ea68e5e/chrome/browser/resources/chromeos/multidevice_setup/multidevice_setup_dialog.js
[add] https://crrev.com/caba515ed1c02bff785651734da65f6ee1516e42/chrome/browser/resources/chromeos/multidevice_setup/multidevice_setup_post_oobe.html
[add] https://crrev.com/caba515ed1c02bff785651734da65f6ee1516e42/chrome/browser/resources/chromeos/multidevice_setup/multidevice_setup_post_oobe.js
[modify] https://crrev.com/caba515ed1c02bff785651734da65f6ee1516e42/chrome/browser/resources/chromeos/multidevice_setup/multidevice_setup_resources.grd
[add] https://crrev.com/caba515ed1c02bff785651734da65f6ee1516e42/chrome/browser/resources/chromeos/multidevice_setup/post_oobe_delegate.html
[add] https://crrev.com/caba515ed1c02bff785651734da65f6ee1516e42/chrome/browser/resources/chromeos/multidevice_setup/post_oobe_delegate.js
[modify] https://crrev.com/caba515ed1c02bff785651734da65f6ee1516e42/chrome/test/data/webui/multidevice_setup/integration_test.js
[modify] https://crrev.com/caba515ed1c02bff785651734da65f6ee1516e42/ui/webui/resources/cr_components/chromeos/multidevice_setup/BUILD.gn
[modify] https://crrev.com/caba515ed1c02bff785651734da65f6ee1516e42/ui/webui/resources/cr_components/chromeos/multidevice_setup/multidevice_setup.html
[modify] https://crrev.com/caba515ed1c02bff785651734da65f6ee1516e42/ui/webui/resources/cr_components/chromeos/multidevice_setup/multidevice_setup.js
[rename] https://crrev.com/caba515ed1c02bff785651734da65f6ee1516e42/ui/webui/resources/cr_components/chromeos/multidevice_setup/multidevice_setup_delegate.html
[add] https://crrev.com/caba515ed1c02bff785651734da65f6ee1516e42/ui/webui/resources/cr_components/chromeos/multidevice_setup/multidevice_setup_delegate.js
[delete] https://crrev.com/ef5f45aebcc6b09dca18c841d43c85498ea68e5e/ui/webui/resources/cr_components/chromeos/multidevice_setup/ui_mode.js
[modify] https://crrev.com/caba515ed1c02bff785651734da65f6ee1516e42/ui/webui/resources/cr_components/cr_components_resources.grdp

Project Member

Comment 8 by bugdroid1@chromium.org, Sep 29

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

commit 57cbbcd9cc392bd00bad619b42e4d90070e1067f
Author: Kyle Horimoto <khorimoto@google.com>
Date: Sat Sep 29 01:10:03 2018

[CrOS MultiDevice] Refactor button usage in MultiDevice setup flow.

Before this change, buttons resided directly within the top-level
<multidevice-setup> element. However, once we integrate this flow with
OOBE, special OOBE-specific buttons must be used, so it is not possible
to embed these buttons directly.

This CL adds <slot>s for both the forward and backward buttons, which
allow each implementation to pass in its own button type. For the
current flow, normal <paper-button>s are passed. A future CL will pass
OOBE-specific buttons when appropriate.

Bug:  884058 
Change-Id: I062bfaa95454e51c0aa6ce19c16ef1a0bca8ab07
Reviewed-on: https://chromium-review.googlesource.com/1237533
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595278}
[modify] https://crrev.com/57cbbcd9cc392bd00bad619b42e4d90070e1067f/chrome/browser/resources/chromeos/multidevice_setup/multidevice_setup_post_oobe.html
[modify] https://crrev.com/57cbbcd9cc392bd00bad619b42e4d90070e1067f/chrome/browser/resources/chromeos/multidevice_setup/multidevice_setup_post_oobe.js
[modify] https://crrev.com/57cbbcd9cc392bd00bad619b42e4d90070e1067f/ui/webui/resources/cr_components/chromeos/multidevice_setup/button_bar.html
[modify] https://crrev.com/57cbbcd9cc392bd00bad619b42e4d90070e1067f/ui/webui/resources/cr_components/chromeos/multidevice_setup/button_bar.js
[modify] https://crrev.com/57cbbcd9cc392bd00bad619b42e4d90070e1067f/ui/webui/resources/cr_components/chromeos/multidevice_setup/multidevice_setup.html
[modify] https://crrev.com/57cbbcd9cc392bd00bad619b42e4d90070e1067f/ui/webui/resources/cr_components/chromeos/multidevice_setup/multidevice_setup.js

Status: Fixed (was: Started)
Project Member

Comment 10 by bugdroid1@chromium.org, Oct 5

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

commit 91113ac2ff506728c77b131ffbc5b61e232a2a1b
Author: Kyle Horimoto <khorimoto@google.com>
Date: Fri Oct 05 22:33:11 2018

[CrOS MultiDevice] Correct two HTML-related issues.

(1) Previously, <multidevice-setup> contained the following line:
    <slot name="forward-button" slot="backward-button"></slot>
This should have read:
    <slot name="forward-button" slot="forward-button"></slot>

(2) Change closing </paper-button> to </div>.

These didn't seem to cause an error as the page still rendered
correctly, but this CL fixes these issues anyway.

Bug:  884058 
Change-Id: Ib7131d3126f664c8dd4f7aac82f94ce0bf28e8dc
Reviewed-on: https://chromium-review.googlesource.com/c/1255048
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597351}
[modify] https://crrev.com/91113ac2ff506728c77b131ffbc5b61e232a2a1b/ui/webui/resources/cr_components/chromeos/multidevice_setup/button_bar.html
[modify] https://crrev.com/91113ac2ff506728c77b131ffbc5b61e232a2a1b/ui/webui/resources/cr_components/chromeos/multidevice_setup/multidevice_setup.html

Sign in to add a comment