Play Store section does not match spec |
||||||||||||||||||||||||||
Issue description1. Row heights do not appear to be standard. 2. "Learn more" link should be adjacent to the text, eg: Enable Google Play Store on your Chromebook. Learn more [policy] [switch] 3. There shouldn't be a "." at the end of "Learn more" 4. "Manage your Android preferences" shouldn't be indented 5. There should be a divider between the two rows
,
Mar 6 2017
Is there a link to the spec handy?
,
Mar 6 2017
,
Mar 6 2017
,
Mar 7 2017
It should be a combination of a few of the existing specs: * cards for padding [1] * rows [2] * management [3] [1] https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20Inner%20Pages/03-Settings/specs#%2FSPEC-settings_cards.png%3Fz=half [2] https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20Inner%20Pages/03-Settings/specs#%2FSPEC-settings_rows.png%3Fz=half [3] https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20Inner%20Pages/03-Settings/specs#%2FSPEC-settings_mgmt.png%3Fz=half
,
Mar 7 2017
Let me know if more detail is needed!
,
Mar 7 2017
,
Mar 8 2017
Issue 696425 has been merged into this issue.
,
Mar 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fbde8588d4780168d7c735491469c0d6b7bc1675 commit fbde8588d4780168d7c735491469c0d6b7bc1675 Author: stevenjb <stevenjb@chromium.org> Date: Tue Mar 14 23:55:02 2017 MD Settings: Fix layout for Play Store 'Learn more' link Partial fix to align items in the row. BUG= 698463 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2755523002 Cr-Commit-Position: refs/heads/master@{#456898} [modify] https://crrev.com/fbde8588d4780168d7c735491469c0d6b7bc1675/chrome/browser/resources/settings/android_apps_page/android_apps_page.html
,
Mar 15 2017
Issue 701671 has been merged into this issue.
,
Mar 27 2017
,
Mar 29 2017
,
Mar 30 2017
Tom/Alan: I posted some questions to the folio mock: 1. When the pref is enforced, should we hide the 'Remove Google Play Store' option, or disable it? 2. Do we want the 'Learn more' link after 'Install apps and games from Google Play'? 3. Is the 'Learn more' link in the 'Remove' dialog the same link as before? (The 'Install Android apps on your Chromebook' support page).
,
Mar 30 2017
Screenshots (non enterprise)
,
Apr 3 2017
,
Apr 3 2017
From bettes@ in a duplicate bug: > Added this line-item to the tracker > https://docs.google.com/spreadsheets/d/1U-nodCUKFrCjvSk5xSRublZkihBGtEMavhRb7BBeDCA/edit?ts=58b896e5#gid=0
,
Apr 3 2017
This looks great! I think we only need a couple string changes: For the top-level row, let's use: Google Play Store Install apps and games from Google Play on your Chromebook. [Learn more] For the dialog, see the attached screenshot for strings to re-use
,
Apr 3 2017
@Steven, I don't necessarily want strings to block Settings from getting to Chrome OS so if reverting back to the old strings, seen in comment 20, are preferred for launch, we should probably do so. Please preserve the line-breaks as seen in the attachment as well. Thank you!
,
Apr 3 2017
bettes@ - You're confusing me. Can you and Tom talk and get this straightened out? The CL has been reviewed but I have to fix some a11y tests. Changing strings at the same time is no problem if we can all agree... :)
,
Apr 3 2017
@stevenjb, we discussed this earlier. We should plan to use the strings in #20 for now. Separately, Alan and I are talking with the ARC++ folks about using the shorter dialog text. It's sensitive so we want to make sure there is buy-in before we change anything. Using the old strings is safest to guarantee M59 launch.
,
Apr 3 2017
OK, thanks for the clarification. I will update to the strings in #20.
,
Apr 3 2017
Also, to confirm, should the section title be "Google Play Store" now, or still "Google Play Store (beta)"? If the former, should we change that in old settings?
,
Apr 3 2017
Remove the "(beta)" in red: - https://screenshot.googleplex.com/Lw9buXxoB2u - https://screenshot.googleplex.com/XXTSMf5DsVd - https://screenshot.googleplex.com/wOEhGqWb8r3 Also, let's keep the subtext the same regardless of whether it's enabled or disabled (i.e. don't change subtext to "On" when enabled)
,
Apr 3 2017
Updated screen shots attached.
,
Apr 3 2017
,
Apr 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/936a64459466aea94d5782255bc0570d9d764dae commit 936a64459466aea94d5782255bc0570d9d764dae Author: stevenjb <stevenjb@chromium.org> Date: Tue Apr 04 20:20:22 2017 MD Settings: Google Play Store: Add subpage and polish See issue for screenshots and details. BUG= 698463 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2785013003 Cr-Commit-Position: refs/heads/master@{#461813} [modify] https://crrev.com/936a64459466aea94d5782255bc0570d9d764dae/chrome/app/settings_strings.grdp [modify] https://crrev.com/936a64459466aea94d5782255bc0570d9d764dae/chrome/browser/resources/settings/android_apps_page/android_apps_page.html [modify] https://crrev.com/936a64459466aea94d5782255bc0570d9d764dae/chrome/browser/resources/settings/android_apps_page/android_apps_page.js [add] https://crrev.com/936a64459466aea94d5782255bc0570d9d764dae/chrome/browser/resources/settings/android_apps_page/android_apps_subpage.html [add] https://crrev.com/936a64459466aea94d5782255bc0570d9d764dae/chrome/browser/resources/settings/android_apps_page/android_apps_subpage.js [modify] https://crrev.com/936a64459466aea94d5782255bc0570d9d764dae/chrome/browser/resources/settings/android_apps_page/compiled_resources2.gyp [modify] https://crrev.com/936a64459466aea94d5782255bc0570d9d764dae/chrome/browser/resources/settings/route.js [modify] https://crrev.com/936a64459466aea94d5782255bc0570d9d764dae/chrome/browser/resources/settings/settings_resources.grd [modify] https://crrev.com/936a64459466aea94d5782255bc0570d9d764dae/chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc [modify] https://crrev.com/936a64459466aea94d5782255bc0570d9d764dae/chrome/test/data/webui/settings/android_apps_page_test.js
,
Apr 4 2017
,
Apr 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/efc241285631bb8f93f709b8cf35c3eee7511ad5 commit efc241285631bb8f93f709b8cf35c3eee7511ad5 Author: kelvinp <kelvinp@chromium.org> Date: Tue Apr 04 22:13:39 2017 Revert of MD Settings: Google Play Store: Add subpage and polish (patchset #4 id:60001 of https://codereview.chromium.org/2785013003/ ) Reason for revert: This change causes 88 browser_tests to fail on Builder Linux ChromiumOS Tests (dbg)(1) Build https://uberchromegw.corp.google.com/i/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%28dbg%29%281%29/builds/24459 Original issue's description: > MD Settings: Google Play Store: Add subpage and polish > > See issue for screenshots and details. > > BUG= 698463 > CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation > > Review-Url: https://codereview.chromium.org/2785013003 > Cr-Commit-Position: refs/heads/master@{#461813} > Committed: https://chromium.googlesource.com/chromium/src/+/936a64459466aea94d5782255bc0570d9d764dae TBR=dbeam@chromium.org,stevenjb@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 698463 Review-Url: https://codereview.chromium.org/2798753002 Cr-Commit-Position: refs/heads/master@{#461864} [modify] https://crrev.com/efc241285631bb8f93f709b8cf35c3eee7511ad5/chrome/app/settings_strings.grdp [modify] https://crrev.com/efc241285631bb8f93f709b8cf35c3eee7511ad5/chrome/browser/resources/settings/android_apps_page/android_apps_page.html [modify] https://crrev.com/efc241285631bb8f93f709b8cf35c3eee7511ad5/chrome/browser/resources/settings/android_apps_page/android_apps_page.js [delete] https://crrev.com/f7d92cff18cba731d3a33d19d3f9595bbacd6fb6/chrome/browser/resources/settings/android_apps_page/android_apps_subpage.html [delete] https://crrev.com/f7d92cff18cba731d3a33d19d3f9595bbacd6fb6/chrome/browser/resources/settings/android_apps_page/android_apps_subpage.js [modify] https://crrev.com/efc241285631bb8f93f709b8cf35c3eee7511ad5/chrome/browser/resources/settings/android_apps_page/compiled_resources2.gyp [modify] https://crrev.com/efc241285631bb8f93f709b8cf35c3eee7511ad5/chrome/browser/resources/settings/route.js [modify] https://crrev.com/efc241285631bb8f93f709b8cf35c3eee7511ad5/chrome/browser/resources/settings/settings_resources.grd [modify] https://crrev.com/efc241285631bb8f93f709b8cf35c3eee7511ad5/chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc [modify] https://crrev.com/efc241285631bb8f93f709b8cf35c3eee7511ad5/chrome/test/data/webui/settings/android_apps_page_test.js
,
Apr 5 2017
@stevenjb re-opening this due to the revert.
,
Apr 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/385710b054844844e583ce48175d2580d16bffd7 commit 385710b054844844e583ce48175d2580d16bffd7 Author: stevenjb <stevenjb@chromium.org> Date: Wed Apr 05 18:04:30 2017 MD Settings: Google Play Store: Add subpage and polish (Take 2) See issue for screenshots and details. Originally landed in https://codereview.chromium.org/2785013003/ Reverted due to browser_test debug build assertiion. BUG= 698463 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2796783005 Cr-Commit-Position: refs/heads/master@{#462143} [modify] https://crrev.com/385710b054844844e583ce48175d2580d16bffd7/chrome/app/settings_strings.grdp [modify] https://crrev.com/385710b054844844e583ce48175d2580d16bffd7/chrome/browser/resources/settings/android_apps_page/android_apps_page.html [modify] https://crrev.com/385710b054844844e583ce48175d2580d16bffd7/chrome/browser/resources/settings/android_apps_page/android_apps_page.js [add] https://crrev.com/385710b054844844e583ce48175d2580d16bffd7/chrome/browser/resources/settings/android_apps_page/android_apps_subpage.html [add] https://crrev.com/385710b054844844e583ce48175d2580d16bffd7/chrome/browser/resources/settings/android_apps_page/android_apps_subpage.js [modify] https://crrev.com/385710b054844844e583ce48175d2580d16bffd7/chrome/browser/resources/settings/android_apps_page/compiled_resources2.gyp [modify] https://crrev.com/385710b054844844e583ce48175d2580d16bffd7/chrome/browser/resources/settings/route.js [modify] https://crrev.com/385710b054844844e583ce48175d2580d16bffd7/chrome/browser/resources/settings/settings_resources.grd [modify] https://crrev.com/385710b054844844e583ce48175d2580d16bffd7/chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc [modify] https://crrev.com/385710b054844844e583ce48175d2580d16bffd7/chrome/test/data/webui/settings/android_apps_page_test.js
,
Apr 5 2017
re-closing since it re-landed :)
,
May 8 2017
Verified on ChromeOS 9532.0.0, 60.0.3092.0 with steps from c#27 |
||||||||||||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||||||||||||
Comment 1 by bettes@chromium.org
, Mar 4 2017