Enable setting language in Demo Mode |
||||||||||||||||||||
Issue descriptionWe need to allow the user to change the language in Demo Mode. 1) In system tray: Show a "Language" button in demo mode. In the expanded view, show a list of supported languages (currently demo mode supports nine). No additional mock/spec is needed. The UI will be consistent with other buttons in the unified system tray. 2) For settings: (unnecessary after the system tray implementation is done?) This setting is normally not enabled in ephemeral sessions, so we'll need to make an exception for Demo Mode public sessions. Repro steps: 1. Open settings => Languages and input 2. Expand Languages, add a language like French 3. Tap the 3-dot menu for the newly added language Expected: checkbox with "Display Chrome OS in this language".
,
Oct 19
,
Oct 19
,
Oct 23
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c7402e82b7718442b8409bb9b1224e0732958cbe commit c7402e82b7718442b8409bb9b1224e0732958cbe Author: Wenzhao Zang <wzang@chromium.org> Date: Tue Oct 23 17:49:17 2018 cros: Allow demo session to change locale and limit it to one session We should allow demo session users to change locale from Settings, which takes effect after a restart. But by the second time the session restarts, the system should revert to the original locale. This CL tries to simulate the following user actions: 1) In the Settings page, you can find the "Currently displayed" locale (A), which comes from the kApplicationLocale in user profile. When you select locale B, the Settings page calls |ChangeAppLocale| but it requires a restart to take effect. Before the restart, there's no visible UI change. 2) After restart B is displayed, now go to Settings and change the locale back to A. 3) After a second restart A is displayed. We can assume the system is now in the same state with 1). So we could save the value A as the original default locale and whenever we detect there's a difference between A and the current locale, call |ChangeAppLocale| to change it back which takes effect in the next session. It's safe to initialize the original default locale with the value A, because there's no way for users to change locale prior to this CL. This is also preferable than getting the value directly from enrollment because: 1) By the time this code is deployed, there're devices already enrolled in demo mode so they can't go through OOBE again. 2) OOBE saves the locale pref in local state, and convert it to a pref in user profile (which Settings page uses). It's safe to be consistent with the Settings page so that the above user actions are simulated. Bug: 877749 Change-Id: Iabbdfcf10fdb19f931c3122c21fd3af792b93191 Reviewed-on: https://chromium-review.googlesource.com/c/1292054 Reviewed-by: Michael Giuffrida <michaelpg@chromium.org> Reviewed-by: Dominic Battré <battre@chromium.org> Reviewed-by: Stefan Kuhne <skuhne@chromium.org> Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org> Cr-Commit-Position: refs/heads/master@{#602011} [modify] https://crrev.com/c7402e82b7718442b8409bb9b1224e0732958cbe/chrome/browser/chromeos/login/demo_mode/demo_session.cc [modify] https://crrev.com/c7402e82b7718442b8409bb9b1224e0732958cbe/chrome/browser/chromeos/login/demo_mode/demo_session.h [modify] https://crrev.com/c7402e82b7718442b8409bb9b1224e0732958cbe/chrome/browser/chromeos/login/demo_mode/demo_session_unittest.cc [modify] https://crrev.com/c7402e82b7718442b8409bb9b1224e0732958cbe/chrome/browser/prefs/browser_prefs.cc [modify] https://crrev.com/c7402e82b7718442b8409bb9b1224e0732958cbe/chrome/browser/profiles/profile.h [modify] https://crrev.com/c7402e82b7718442b8409bb9b1224e0732958cbe/chrome/browser/resources/settings/device_page/device_page.js [modify] https://crrev.com/c7402e82b7718442b8409bb9b1224e0732958cbe/chrome/browser/resources/settings/languages_page/languages_page.js [modify] https://crrev.com/c7402e82b7718442b8409bb9b1224e0732958cbe/chrome/browser/ui/webui/settings/languages_handler.cc [modify] https://crrev.com/c7402e82b7718442b8409bb9b1224e0732958cbe/chrome/browser/ui/webui/settings/md_settings_ui.cc [modify] https://crrev.com/c7402e82b7718442b8409bb9b1224e0732958cbe/chrome/common/pref_names.cc [modify] https://crrev.com/c7402e82b7718442b8409bb9b1224e0732958cbe/chrome/common/pref_names.h [modify] https://crrev.com/c7402e82b7718442b8409bb9b1224e0732958cbe/chrome/test/base/testing_profile.cc [modify] https://crrev.com/c7402e82b7718442b8409bb9b1224e0732958cbe/chrome/test/base/testing_profile.h
,
Nov 7
,
Nov 7
,
Nov 7
,
Nov 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0cb311d76efa04621ac8dd6089e26e984d040c08 commit 0cb311d76efa04621ac8dd6089e26e984d040c08 Author: Wenzhao Zang <wzang@chromium.org> Date: Thu Nov 15 22:42:31 2018 cros: Enable changing locale from system tray in demo mode Show a locale feature button in the unified system tray in demo mode. The UI is implemented in //ash/system, but the data should come from //chrome. The mock is in go/CrosDemoLanguage. No additional spec is needed: the UI is consistent with other buttons in the system tray. The change is approved by UI review. Bug: 877749 Change-Id: I23a88584845e432b7762aeac03ab86d659cca07c Reviewed-on: https://chromium-review.googlesource.com/c/1321775 Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org> Reviewed-by: Steven Bennetts <stevenjb@chromium.org> Reviewed-by: Stefan Kuhne <skuhne@chromium.org> Reviewed-by: Oliver Chang <ochang@chromium.org> Reviewed-by: Michael Giuffrida <michaelpg@chromium.org> Reviewed-by: Tetsui Ohkubo <tetsui@chromium.org> Cr-Commit-Position: refs/heads/master@{#608557} [modify] https://crrev.com/0cb311d76efa04621ac8dd6089e26e984d040c08/ash/BUILD.gn [modify] https://crrev.com/0cb311d76efa04621ac8dd6089e26e984d040c08/ash/ash_strings.grd [modify] https://crrev.com/0cb311d76efa04621ac8dd6089e26e984d040c08/ash/public/interfaces/system_tray.mojom [add] https://crrev.com/0cb311d76efa04621ac8dd6089e26e984d040c08/ash/system/locale/locale_detailed_view.cc [add] https://crrev.com/0cb311d76efa04621ac8dd6089e26e984d040c08/ash/system/locale/locale_detailed_view.h [add] https://crrev.com/0cb311d76efa04621ac8dd6089e26e984d040c08/ash/system/locale/locale_feature_pod_controller.cc [add] https://crrev.com/0cb311d76efa04621ac8dd6089e26e984d040c08/ash/system/locale/locale_feature_pod_controller.h [add] https://crrev.com/0cb311d76efa04621ac8dd6089e26e984d040c08/ash/system/locale/locale_feature_pod_controller_unittest.cc [add] https://crrev.com/0cb311d76efa04621ac8dd6089e26e984d040c08/ash/system/locale/unified_locale_detailed_view_controller.cc [add] https://crrev.com/0cb311d76efa04621ac8dd6089e26e984d040c08/ash/system/locale/unified_locale_detailed_view_controller.h [modify] https://crrev.com/0cb311d76efa04621ac8dd6089e26e984d040c08/ash/system/model/system_tray_model.cc [modify] https://crrev.com/0cb311d76efa04621ac8dd6089e26e984d040c08/ash/system/model/system_tray_model.h [modify] https://crrev.com/0cb311d76efa04621ac8dd6089e26e984d040c08/ash/system/tray/system_tray_item_uma_type.h [modify] https://crrev.com/0cb311d76efa04621ac8dd6089e26e984d040c08/ash/system/unified/unified_system_tray_controller.cc [modify] https://crrev.com/0cb311d76efa04621ac8dd6089e26e984d040c08/ash/system/unified/unified_system_tray_controller.h [modify] https://crrev.com/0cb311d76efa04621ac8dd6089e26e984d040c08/chrome/browser/chromeos/login/demo_mode/demo_session.cc [modify] https://crrev.com/0cb311d76efa04621ac8dd6089e26e984d040c08/chrome/browser/profiles/profile.h [modify] https://crrev.com/0cb311d76efa04621ac8dd6089e26e984d040c08/chrome/browser/ui/ash/system_tray_client.cc [modify] https://crrev.com/0cb311d76efa04621ac8dd6089e26e984d040c08/chrome/browser/ui/ash/system_tray_client.h [modify] https://crrev.com/0cb311d76efa04621ac8dd6089e26e984d040c08/tools/metrics/histograms/enums.xml
,
Nov 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/106f739376e319eba280bf7bc9440c8f96f47444 commit 106f739376e319eba280bf7bc9440c8f96f47444 Author: Wenzhao Zang <wzang@chromium.org> Date: Fri Nov 16 01:02:45 2018 cros: Add tooltip and icon to the system tray locale button The screenshot can be found at https://drive.google.com/file/d/1zMTMCNiJQXcLzeL_sfZaWNMS-OJ5n7SM/view?usp=sharing Also include the hash in the CL. Bug: 877749 Change-Id: Ibf118e2c20f158cf2fd2c2cef955c9d04c205b86 Reviewed-on: https://chromium-review.googlesource.com/c/1333830 Reviewed-by: Evan Stade <estade@chromium.org> Reviewed-by: Tetsui Ohkubo <tetsui@chromium.org> Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org> Cr-Commit-Position: refs/heads/master@{#608608} [modify] https://crrev.com/106f739376e319eba280bf7bc9440c8f96f47444/ash/ash_strings.grd [add] https://crrev.com/106f739376e319eba280bf7bc9440c8f96f47444/ash/ash_strings_grd/IDS_ASH_STATUS_TRAY_LOCALE_TOOLTIP.png.sha1 [modify] https://crrev.com/106f739376e319eba280bf7bc9440c8f96f47444/ash/resources/vector_icons/BUILD.gn [add] https://crrev.com/106f739376e319eba280bf7bc9440c8f96f47444/ash/resources/vector_icons/unified_menu_locale.icon [modify] https://crrev.com/106f739376e319eba280bf7bc9440c8f96f47444/ash/system/locale/locale_feature_pod_controller.cc
,
Nov 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a77427560f0ad5a9be9f8ae9d855f0d70a34c389 commit a77427560f0ad5a9be9f8ae9d855f0d70a34c389 Author: Wenzhao Zang <wzang@chromium.org> Date: Fri Nov 16 01:37:50 2018 cros: Remove the option to change locale from Settings during Demo Mode It's unnecessary after the system tray button UI is added. Also the locale list in Settings is not customized for Demo Mode. Bug: 877749 Change-Id: I17c0ab8cab457a5a47416797677ce3b17d9b4b1b Reviewed-on: https://chromium-review.googlesource.com/c/1334333 Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org> Reviewed-by: Michael Giuffrida <michaelpg@chromium.org> Cr-Commit-Position: refs/heads/master@{#608620} [modify] https://crrev.com/a77427560f0ad5a9be9f8ae9d855f0d70a34c389/chrome/browser/resources/settings/languages_page/languages_page.js [modify] https://crrev.com/a77427560f0ad5a9be9f8ae9d855f0d70a34c389/chrome/browser/ui/webui/settings/languages_handler.cc
,
Nov 21
Hi Olga and Elizabeth, I'm reusing the spec of the "Keyboard" button on the new "Language" button, however as the screenshots show, the font for "en-US" is shrunk to a small size and it may not be legible. The "keyboard" button seems to be free of this problem, because it doesn't have a string that's as long as "en-US". tetsui@ raised this concern when doing the code review. Do we want to use a bigger width? But please note that it will further squeeze the space for the full name, which already overflows. (IMO the current font may be OK because the short name is more for decoration purpose than being functional.)
,
Nov 25
Regarding #11, my concern is in some cases the font size of these strings e.g. "en-US" might be too small, so that they might not be compatible with a11y guidelines.
,
Nov 26
Elizabeth - can you please help with specs?
,
Nov 28
According to Elizabeth and other UXers, we'll display the first two letters only (i.e. "en-US" will be cropped to "en"). And it's fine to have multiple "en" because people will look at the long descriptive name anyway.
,
Nov 28
,
Nov 28
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8f6850d62ee3381ad5b9de99d860b59442bcfcdb commit 8f6850d62ee3381ad5b9de99d860b59442bcfcdb Author: Wenzhao Zang <wzang@chromium.org> Date: Wed Nov 28 02:41:57 2018 cros: Show ISO code at the beginning of each locale item Spent some time trying to extract and reuse the |ImeListItemView|, but it's not easy since it contains IME specific logic (e.g. |set_last_item_selected_with_keyboard|). In fact I believe we should try to combine the class with |HoverHighlightView|, but it's a somewhat large refactoring and is beyond the scope of this CL. Bug: 877749 Change-Id: I9f4582fdcf865da7b3960a625cedbbdf710c7e4f Reviewed-on: https://chromium-review.googlesource.com/c/1341065 Reviewed-by: Tetsui Ohkubo <tetsui@chromium.org> Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org> Cr-Commit-Position: refs/heads/master@{#611530} [modify] https://crrev.com/8f6850d62ee3381ad5b9de99d860b59442bcfcdb/ash/system/locale/locale_detailed_view.cc [modify] https://crrev.com/8f6850d62ee3381ad5b9de99d860b59442bcfcdb/ash/system/locale/locale_detailed_view.h [modify] https://crrev.com/8f6850d62ee3381ad5b9de99d860b59442bcfcdb/ash/system/locale/locale_feature_pod_controller.cc
,
Nov 28
,
Dec 4
Reopen to track punting the feature to M73, and adding the language icon in the status area.
,
Dec 4
,
Dec 5
Olga - the abbreviation for the keyboard (e.g. US) may be shown in the status area. So if we show another language code there as go/CrosDemoLanguage suggests, there will be confusion. See screenshot.
,
Dec 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9827fd8ee983905295fc3ead5550f20165df1809 commit 9827fd8ee983905295fc3ead5550f20165df1809 Author: Wenzhao Zang <wzang@chromium.org> Date: Mon Dec 10 18:41:37 2018 cros: Temporarily disable system tray language toggle in demo mode Disable the feature in M72 because I overlooked the requirement for adding the visual indicator in status area. At this point, users have no easy way to know the language toggle is supported, unless they open the system tray themselves (something that customers in retail store rarely do). PM decides to disable the feature altogether. Bug: 877749 Change-Id: I7f619caaea18be43ebc2631d2fb1eefbf8d6a45f Reviewed-on: https://chromium-review.googlesource.com/c/1359858 Commit-Queue: Michael Giuffrida <michaelpg@chromium.org> Reviewed-by: Michael Giuffrida <michaelpg@chromium.org> Cr-Commit-Position: refs/heads/master@{#615185} [modify] https://crrev.com/9827fd8ee983905295fc3ead5550f20165df1809/chrome/browser/chromeos/login/demo_mode/demo_session.cc [modify] https://crrev.com/9827fd8ee983905295fc3ead5550f20165df1809/chromeos/chromeos_switches.cc [modify] https://crrev.com/9827fd8ee983905295fc3ead5550f20165df1809/chromeos/chromeos_switches.h
,
Dec 10
Request for CL in #21.
,
Dec 11
This bug requires manual review: There is .grd file changes and we are only 48 days from stable. Please contact the milestone owner if you have questions. Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 11
To clarify: the CL in #21 does not contain .grd file.
,
Dec 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bdf773451b089586e796156461d594198196930d commit bdf773451b089586e796156461d594198196930d Author: Wenzhao Zang <wzang@chromium.org> Date: Wed Dec 12 00:24:45 2018 cros: Add current locale icon to status tray We want to show a visual indicator in the status tray when the language toggle is available (in demo mode). It should show the abbreviation of the locale such as "DA". The only caveat is that the IME indicator may also be shown in the same place, so showing two strings with the same format may confuse users. PM confirms that we want to hide the IME indicator as long as the locale indicator is available. Bug: 877749 Change-Id: Ieca21dd46c846b81147bd44798b2deb25d9d57fa Reviewed-on: https://chromium-review.googlesource.com/c/1370886 Reviewed-by: Tetsui Ohkubo <tetsui@chromium.org> Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org> Cr-Commit-Position: refs/heads/master@{#615740} [modify] https://crrev.com/bdf773451b089586e796156461d594198196930d/ash/BUILD.gn [modify] https://crrev.com/bdf773451b089586e796156461d594198196930d/ash/ash_strings.grd [add] https://crrev.com/bdf773451b089586e796156461d594198196930d/ash/ash_strings_grd/IDS_ASH_STATUS_TRAY_INDICATOR_LOCALE_TOOLTIP.png.sha1 [modify] https://crrev.com/bdf773451b089586e796156461d594198196930d/ash/system/locale/locale_detailed_view.cc [modify] https://crrev.com/bdf773451b089586e796156461d594198196930d/ash/system/locale/locale_feature_pod_controller.cc [add] https://crrev.com/bdf773451b089586e796156461d594198196930d/ash/system/model/locale_model.cc [add] https://crrev.com/bdf773451b089586e796156461d594198196930d/ash/system/model/locale_model.h [modify] https://crrev.com/bdf773451b089586e796156461d594198196930d/ash/system/model/system_tray_model.cc [modify] https://crrev.com/bdf773451b089586e796156461d594198196930d/ash/system/model/system_tray_model.h [add] https://crrev.com/bdf773451b089586e796156461d594198196930d/ash/system/unified/current_locale_view.cc [add] https://crrev.com/bdf773451b089586e796156461d594198196930d/ash/system/unified/current_locale_view.h [modify] https://crrev.com/bdf773451b089586e796156461d594198196930d/ash/system/unified/ime_mode_view.cc [modify] https://crrev.com/bdf773451b089586e796156461d594198196930d/ash/system/unified/ime_mode_view.h [modify] https://crrev.com/bdf773451b089586e796156461d594198196930d/ash/system/unified/unified_system_tray.cc [modify] https://crrev.com/bdf773451b089586e796156461d594198196930d/ash/system/unified/unified_system_tray.h
,
Dec 12
,
Dec 13
cc'ed PTM djmm@ for merge request of the CL in #21. The CL disables a newly introduced feature. The merge risk is very low. Please let me know if you have concerns.
,
Dec 13
,
Dec 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/43a6389205d4437d563cfbe6fcce0a6c908a1cac commit 43a6389205d4437d563cfbe6fcce0a6c908a1cac Author: Wenzhao Zang <wzang@chromium.org> Date: Thu Dec 13 22:52:06 2018 cros: Temporarily disable system tray language toggle in demo mode Disable the feature in M72 because I overlooked the requirement for adding the visual indicator in status area. At this point, users have no easy way to know the language toggle is supported, unless they open the system tray themselves (something that customers in retail store rarely do). PM decides to disable the feature altogether. Bug: 877749 Change-Id: I7f619caaea18be43ebc2631d2fb1eefbf8d6a45f Reviewed-on: https://chromium-review.googlesource.com/c/1359858 Commit-Queue: Michael Giuffrida <michaelpg@chromium.org> Reviewed-by: Michael Giuffrida <michaelpg@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#615185}(cherry picked from commit 9827fd8ee983905295fc3ead5550f20165df1809) Reviewed-on: https://chromium-review.googlesource.com/c/1377181 Reviewed-by: Wenzhao (Colin) Zang <wzang@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#345} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} [modify] https://crrev.com/43a6389205d4437d563cfbe6fcce0a6c908a1cac/chrome/browser/chromeos/login/demo_mode/demo_session.cc [modify] https://crrev.com/43a6389205d4437d563cfbe6fcce0a6c908a1cac/chromeos/chromeos_switches.cc [modify] https://crrev.com/43a6389205d4437d563cfbe6fcce0a6c908a1cac/chromeos/chromeos_switches.h
,
Dec 19
Here's a summary of the rules that were executed: - OnlyMergeApprovedChange: Rule Failed -- Revision 43a6389205d4437d563cfbe6fcce0a6c908a1cac was merged to refs/branch-heads/3626 branch with no merge approval from a TPM! Please explain why this change was merged to the branch! - AcknowledgeMerge: Notification Required --
,
Dec 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/43a6389205d4437d563cfbe6fcce0a6c908a1cac Commit: 43a6389205d4437d563cfbe6fcce0a6c908a1cac Author: wzang@chromium.org Commiter: wzang@chromium.org Date: 2018-12-13 22:52:06 +0000 UTC cros: Temporarily disable system tray language toggle in demo mode Disable the feature in M72 because I overlooked the requirement for adding the visual indicator in status area. At this point, users have no easy way to know the language toggle is supported, unless they open the system tray themselves (something that customers in retail store rarely do). PM decides to disable the feature altogether. Bug: 877749 Change-Id: I7f619caaea18be43ebc2631d2fb1eefbf8d6a45f Reviewed-on: https://chromium-review.googlesource.com/c/1359858 Commit-Queue: Michael Giuffrida <michaelpg@chromium.org> Reviewed-by: Michael Giuffrida <michaelpg@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#615185}(cherry picked from commit 9827fd8ee983905295fc3ead5550f20165df1809) Reviewed-on: https://chromium-review.googlesource.com/c/1377181 Reviewed-by: Wenzhao (Colin) Zang <wzang@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#345} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
,
Dec 28
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d340e95ef211c73e8d97f8e4241e288abc99cbbf commit d340e95ef211c73e8d97f8e4241e288abc99cbbf Author: Wenzhao Zang <wzang@chromium.org> Date: Fri Dec 28 23:37:10 2018 cros: Enable language toggle in Demo Mode by default Bug: 877749 Change-Id: I72368a6958a8b24dedb3de5404f133afbc3d0307 Reviewed-on: https://chromium-review.googlesource.com/c/1392328 Reviewed-by: Michael Giuffrida <michaelpg@chromium.org> Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org> Cr-Commit-Position: refs/heads/master@{#619205} [modify] https://crrev.com/d340e95ef211c73e8d97f8e4241e288abc99cbbf/chromeos/chromeos_switches.cc |
||||||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||||||
Comment 1 by wzang@chromium.org
, Aug 27