Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2504233004:
Tweaks for: Toggle spoken feedback if two fingers are held down (Closed)
|
Created:
4 years, 1 month ago by dmazzoni Modified:
4 years ago CC:
chromium-reviews, oshima+watch_chromium.org, aboxhall+watch_chromium.org, nektar+watch_chromium.org, yuzo+watch_chromium.org, asvitkine+watch_chromium.org, je_julie, dmazzoni+watch_chromium.org, dtseng+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionTweaks for: Toggle spoken feedback if two fingers are held down
Change r432129 added a feature to toggle spoken feedback if the user
holds down two fingers for a length of time.
Based on feedback from UI review, don't play any audio feedback until
3.0 seconds, and then toggle at 5.0 seconds.
Also record an actiontracking the fact that the audio feedback
occurred at 3.0 seconds - we can compare that with the existing metric
for toggling spoken feedback to see how often users are triggering the
warning sound and how often they're triggering spoken feedback on
purpose or by accident.
BUG=662501
Committed: https://crrev.com/e0151acedf182d1c16ccbe9ba4f56b10e8a0314d
Cr-Commit-Position: refs/heads/master@{#434618}
Patch Set 1 #
Total comments: 6
Patch Set 2 : Add curly braces #Patch Set 3 : Fix spacing #Patch Set 4 : Update unit tests #
Messages
Total messages: 25 (12 generated)
dmazzoni@chromium.org changed reviewers: + isherman@chromium.org, oshima@chromium.org
Metrics lgtm % nits: https://codereview.chromium.org/2504233004/diff/1/tools/metrics/actions/actio... File tools/metrics/actions/actions.xml (right): https://codereview.chromium.org/2504233004/diff/1/tools/metrics/actions/actio... tools/metrics/actions/actions.xml:1002: Metric recorded on Chrome OS when the user holds down two fingers on a nit: Please remove the extra spaces in this description, such as the ones between "down" and "two" https://codereview.chromium.org/2504233004/diff/1/ui/chromeos/touch_accessibi... File ui/chromeos/touch_accessibility_enabler.cc (right): https://codereview.chromium.org/2504233004/diff/1/ui/chromeos/touch_accessibi... ui/chromeos/touch_accessibility_enabler.cc:143: if (tick_count == kTimerTicksOfFirstSoundFeedback) nit: Please add curly braces, since the body of this if-stmt spans multiple lines.
lgtm https://codereview.chromium.org/2504233004/diff/1/ui/chromeos/touch_accessibi... File ui/chromeos/touch_accessibility_enabler.cc (right): https://codereview.chromium.org/2504233004/diff/1/ui/chromeos/touch_accessibi... ui/chromeos/touch_accessibility_enabler.cc:143: if (tick_count == kTimerTicksOfFirstSoundFeedback) On 2016/11/16 23:11:33, Ilya Sherman wrote: > nit: Please add curly braces, since the body of this if-stmt spans multiple > lines. +1
The CQ bit was checked by dmazzoni@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from oshima@chromium.org, isherman@chromium.org Link to the patchset: https://codereview.chromium.org/2504233004/#ps20001 (title: "Add curly braces")
https://codereview.chromium.org/2504233004/diff/1/ui/chromeos/touch_accessibi... File ui/chromeos/touch_accessibility_enabler.cc (right): https://codereview.chromium.org/2504233004/diff/1/ui/chromeos/touch_accessibi... ui/chromeos/touch_accessibility_enabler.cc:143: if (tick_count == kTimerTicksOfFirstSoundFeedback) On 2016/11/22 21:57:38, oshima wrote: > On 2016/11/16 23:11:33, Ilya Sherman wrote: > > nit: Please add curly braces, since the body of this if-stmt spans multiple > > lines. > > +1 Done.
https://codereview.chromium.org/2504233004/diff/1/tools/metrics/actions/actio... File tools/metrics/actions/actions.xml (right): https://codereview.chromium.org/2504233004/diff/1/tools/metrics/actions/actio... tools/metrics/actions/actions.xml:1002: Metric recorded on Chrome OS when the user holds down two fingers on a On 2016/11/16 23:11:33, Ilya Sherman wrote: > nit: Please remove the extra spaces in this description, such as the ones > between "down" and "two" ^^^
https://codereview.chromium.org/2504233004/diff/1/tools/metrics/actions/actio... File tools/metrics/actions/actions.xml (right): https://codereview.chromium.org/2504233004/diff/1/tools/metrics/actions/actio... tools/metrics/actions/actions.xml:1002: Metric recorded on Chrome OS when the user holds down two fingers on a On 2016/11/22 22:33:08, Ilya Sherman wrote: > On 2016/11/16 23:11:33, Ilya Sherman wrote: > > nit: Please remove the extra spaces in this description, such as the ones > > between "down" and "two" > > ^^^ Oops, done.
The CQ bit was checked by dmazzoni@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from oshima@chromium.org, isherman@chromium.org Link to the patchset: https://codereview.chromium.org/2504233004/#ps40001 (title: "Fix spacing")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by dmazzoni@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from oshima@chromium.org, isherman@chromium.org Link to the patchset: https://codereview.chromium.org/2504233004/#ps60001 (title: "Update unit tests")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromeos_x86-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by dmazzoni@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": 60001, "attempt_start_ts": 1480320556068800,
"parent_rev": "1ef7fb7b5a645a23c948483a1fdf8ef1c5a9038c", "commit_rev":
"e0d391f8352a1cbf3d97ea4d1e5943f1bb254031"}
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Tweaks for: Toggle spoken feedback if two fingers are held down Change r432129 added a feature to toggle spoken feedback if the user holds down two fingers for a length of time. Based on feedback from UI review, don't play any audio feedback until 3.0 seconds, and then toggle at 5.0 seconds. Also record an actiontracking the fact that the audio feedback occurred at 3.0 seconds - we can compare that with the existing metric for toggling spoken feedback to see how often users are triggering the warning sound and how often they're triggering spoken feedback on purpose or by accident. BUG=662501 ========== to ========== Tweaks for: Toggle spoken feedback if two fingers are held down Change r432129 added a feature to toggle spoken feedback if the user holds down two fingers for a length of time. Based on feedback from UI review, don't play any audio feedback until 3.0 seconds, and then toggle at 5.0 seconds. Also record an actiontracking the fact that the audio feedback occurred at 3.0 seconds - we can compare that with the existing metric for toggling spoken feedback to see how often users are triggering the warning sound and how often they're triggering spoken feedback on purpose or by accident. BUG=662501 Committed: https://crrev.com/e0151acedf182d1c16ccbe9ba4f56b10e8a0314d Cr-Commit-Position: refs/heads/master@{#434618} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/e0151acedf182d1c16ccbe9ba4f56b10e8a0314d Cr-Commit-Position: refs/heads/master@{#434618} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
