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

Issue 884830 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Oct 24
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Make Instant tether Subpage Item match the native feature-item specs in MultiDevice settings UI

Project Member Reported by jordynass@chromium.org, Sep 17

Issue description

There are a few formatting disparities resulting from is a result of the fact that we needed to use the network-summary-item:

-The toggle is not disabled when the suite is off: https://screenshot.googleplex.com/onUYhRc9Pnw
-The subtext is a description of the network status, not the permanent feature description in the string doc


 
Components: -UI>ProximityAuth UI>Multidevice
Owner: jordynass@chromium.org
Status: Started (was: Available)
Cc: jessejames@chromium.org jlklein@chromium.org lesliewatkins@chromium.org jordynass@chromium.org khorimoto@chromium.org shibasheikh@chromium.org elizabethchiu@chromium.org jhawkins@chromium.org hansberry@chromium.org
This task is now under design discussion because there is substantial ambiguity over the function of the toggle, the relationship of the MultiDevice supage 'instant tethering' item with the 'mobile data' item in the network (aka internet) page, and the role of network status update in the MultiDevice 'instant tethering' item.

I will update this bug with more detail after the branch.
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 24

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

commit 6b1ff066b5f169587b4693f0b295745383e10ea2
Author: Kyle Horimoto <khorimoto@google.com>
Date: Wed Oct 24 01:04:00 2018

[CrOS MultiDevice] Update Instant Tethering item (multi-device subpage).

Before this CL, the multi-device settings displayed Instant Tethering
settings using exactly the same mechanism used by the network settings.
However, this is undesirable because (1) it does not display the
"prohibited" icon when a device administrator prohibits the feature and
(2) it does not display the correct text as a label for the item.

This CL changes the settings subpage to use the same infrastructure as
the rest of the features. As a result, this also requires that
TetherService be updated; previously, TetherService was responsible for
changing the "enabled" user pref itself, but now it needs to support that
mode as well as responding to changes of the "enabled" pref.

Bug:  896324 ,  884830 
Change-Id: I083a8e8e07725130058db35ee540dd6ad733d421
Reviewed-on: https://chromium-review.googlesource.com/c/1297594
Reviewed-by: Tommy Li <tommycli@chromium.org>
Reviewed-by: Jeremy Klein <jlklein@chromium.org>
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602199}
[modify] https://crrev.com/6b1ff066b5f169587b4693f0b295745383e10ea2/chrome/browser/chromeos/tether/tether_service.cc
[modify] https://crrev.com/6b1ff066b5f169587b4693f0b295745383e10ea2/chrome/browser/chromeos/tether/tether_service.h
[modify] https://crrev.com/6b1ff066b5f169587b4693f0b295745383e10ea2/chrome/browser/chromeos/tether/tether_service_unittest.cc
[modify] https://crrev.com/6b1ff066b5f169587b4693f0b295745383e10ea2/chrome/browser/resources/settings/multidevice_page/BUILD.gn
[modify] https://crrev.com/6b1ff066b5f169587b4693f0b295745383e10ea2/chrome/browser/resources/settings/multidevice_page/multidevice_feature_behavior.js
[modify] https://crrev.com/6b1ff066b5f169587b4693f0b295745383e10ea2/chrome/browser/resources/settings/multidevice_page/multidevice_feature_item.html
[modify] https://crrev.com/6b1ff066b5f169587b4693f0b295745383e10ea2/chrome/browser/resources/settings/multidevice_page/multidevice_feature_item.js
[modify] https://crrev.com/6b1ff066b5f169587b4693f0b295745383e10ea2/chrome/browser/resources/settings/multidevice_page/multidevice_subpage.html
[modify] https://crrev.com/6b1ff066b5f169587b4693f0b295745383e10ea2/chrome/browser/resources/settings/multidevice_page/multidevice_subpage.js
[modify] https://crrev.com/6b1ff066b5f169587b4693f0b295745383e10ea2/chrome/browser/resources/settings/multidevice_page/multidevice_tether_item.html
[modify] https://crrev.com/6b1ff066b5f169587b4693f0b295745383e10ea2/chrome/browser/resources/settings/multidevice_page/multidevice_tether_item.js
[modify] https://crrev.com/6b1ff066b5f169587b4693f0b295745383e10ea2/chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 24

Labels: merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bad68c8dd887cab1dcf1efe4a4318c71ac52ed18

commit bad68c8dd887cab1dcf1efe4a4318c71ac52ed18
Author: Kyle Horimoto <khorimoto@google.com>
Date: Wed Oct 24 18:06:34 2018

[CrOS MultiDevice] Update Instant Tethering item (multi-device subpage).

Before this CL, the multi-device settings displayed Instant Tethering
settings using exactly the same mechanism used by the network settings.
However, this is undesirable because (1) it does not display the
"prohibited" icon when a device administrator prohibits the feature and
(2) it does not display the correct text as a label for the item.

This CL changes the settings subpage to use the same infrastructure as
the rest of the features. As a result, this also requires that
TetherService be updated; previously, TetherService was responsible for
changing the "enabled" user pref itself, but now it needs to support that
mode as well as responding to changes of the "enabled" pref.

Bug:  896324 ,  884830 
Change-Id: I083a8e8e07725130058db35ee540dd6ad733d421
Reviewed-on: https://chromium-review.googlesource.com/c/1297594
Reviewed-by: Tommy Li <tommycli@chromium.org>
Reviewed-by: Jeremy Klein <jlklein@chromium.org>
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#602199}(cherry picked from commit 6b1ff066b5f169587b4693f0b295745383e10ea2)
Reviewed-on: https://chromium-review.googlesource.com/c/1298293
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#298}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/bad68c8dd887cab1dcf1efe4a4318c71ac52ed18/chrome/browser/chromeos/tether/tether_service.cc
[modify] https://crrev.com/bad68c8dd887cab1dcf1efe4a4318c71ac52ed18/chrome/browser/chromeos/tether/tether_service.h
[modify] https://crrev.com/bad68c8dd887cab1dcf1efe4a4318c71ac52ed18/chrome/browser/chromeos/tether/tether_service_unittest.cc
[modify] https://crrev.com/bad68c8dd887cab1dcf1efe4a4318c71ac52ed18/chrome/browser/resources/settings/multidevice_page/BUILD.gn
[modify] https://crrev.com/bad68c8dd887cab1dcf1efe4a4318c71ac52ed18/chrome/browser/resources/settings/multidevice_page/multidevice_feature_behavior.js
[modify] https://crrev.com/bad68c8dd887cab1dcf1efe4a4318c71ac52ed18/chrome/browser/resources/settings/multidevice_page/multidevice_feature_item.html
[modify] https://crrev.com/bad68c8dd887cab1dcf1efe4a4318c71ac52ed18/chrome/browser/resources/settings/multidevice_page/multidevice_feature_item.js
[modify] https://crrev.com/bad68c8dd887cab1dcf1efe4a4318c71ac52ed18/chrome/browser/resources/settings/multidevice_page/multidevice_subpage.html
[modify] https://crrev.com/bad68c8dd887cab1dcf1efe4a4318c71ac52ed18/chrome/browser/resources/settings/multidevice_page/multidevice_subpage.js
[modify] https://crrev.com/bad68c8dd887cab1dcf1efe4a4318c71ac52ed18/chrome/browser/resources/settings/multidevice_page/multidevice_tether_item.html
[modify] https://crrev.com/bad68c8dd887cab1dcf1efe4a4318c71ac52ed18/chrome/browser/resources/settings/multidevice_page/multidevice_tether_item.js
[modify] https://crrev.com/bad68c8dd887cab1dcf1efe4a4318c71ac52ed18/chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc

Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/bad68c8dd887cab1dcf1efe4a4318c71ac52ed18

Commit: bad68c8dd887cab1dcf1efe4a4318c71ac52ed18
Author: khorimoto@google.com
Commiter: khorimoto@chromium.org
Date: 2018-10-24 18:06:34 +0000 UTC

[CrOS MultiDevice] Update Instant Tethering item (multi-device subpage).

Before this CL, the multi-device settings displayed Instant Tethering
settings using exactly the same mechanism used by the network settings.
However, this is undesirable because (1) it does not display the
"prohibited" icon when a device administrator prohibits the feature and
(2) it does not display the correct text as a label for the item.

This CL changes the settings subpage to use the same infrastructure as
the rest of the features. As a result, this also requires that
TetherService be updated; previously, TetherService was responsible for
changing the "enabled" user pref itself, but now it needs to support that
mode as well as responding to changes of the "enabled" pref.

Bug:  896324 ,  884830 
Change-Id: I083a8e8e07725130058db35ee540dd6ad733d421
Reviewed-on: https://chromium-review.googlesource.com/c/1297594
Reviewed-by: Tommy Li <tommycli@chromium.org>
Reviewed-by: Jeremy Klein <jlklein@chromium.org>
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#602199}(cherry picked from commit 6b1ff066b5f169587b4693f0b295745383e10ea2)
Reviewed-on: https://chromium-review.googlesource.com/c/1298293
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#298}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
Status: Fixed (was: Started)

Sign in to add a comment