Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
16336002:
Adjust the WebUI keyboard css and layout to work better in narrow spaces. (Closed)
|
Created:
7 years, 6 months ago by SteveT Modified:
7 years, 6 months ago CC:
chromium-reviews Base URL:
http://git.chromium.org/chromium/src.git@master Visibility:
Public. |
DescriptionAdjust the WebUI keyboard css and layout to work better in narrow spaces.
NOTE that this must be patched on top of https://codereview.chromium.org/15176004/
TODOs: Add a list of adjustments here before review.
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=207982
Patch Set 1 #
Total comments: 16
Patch Set 2 : char = Invalid #Patch Set 3 : big Rebase #Patch Set 4 : rebase again #Patch Set 5 : Compromise padding #
Messages
Total messages: 18 (0 generated)
drive by: https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... File ui/keyboard/resources/webui/keysets.html (right): https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... ui/keyboard/resources/webui/keysets.html:22: <kb-key class="left-shift padded-left-special dark" toKeyset="lower" char="Shift">shift</kb-key><kb-key>Z</kb-key> Looks like the char here should be "Invalid". Otherwise, it will send "Shift" characters. This is probably result from rebase. https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... ui/keyboard/resources/webui/keysets.html:28: <kb-key class="symbol dark" toKeyset="symbol">?123</kb-key> Need to add char="Invalid" here too. https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... ui/keyboard/resources/webui/keysets.html:51: <kb-key class="left-shift padded-left-special dark" toKeyset="upper" char="Shift">shift</kb-key><kb-key>z</kb-key> Same here. s/Shift/Invalid https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... ui/keyboard/resources/webui/keysets.html:57: <kb-key class="symbol dark" toKeyset="symbol">?123</kb-key> Add char="Invalid" https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... ui/keyboard/resources/webui/keysets.html:78: <kb-key class="left-more padded-left-special dark" toKeyset="more">more</kb-key> add char="Invalid" https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... ui/keyboard/resources/webui/keysets.html:84: <kb-key class="symbol dark" toKeyset="lower">abc</kb-key> add char="Invalid" https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... ui/keyboard/resources/webui/keysets.html:106: <kb-key class="left-more padded-left-special dark" toKeyset="symbol">?123</kb-key><kb-key></kb-key> add char="Invalid" https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... ui/keyboard/resources/webui/keysets.html:112: <kb-key class="symbol dark" toKeyset="lower">abc</kb-key> add char="Invalid"
Thanks! I think that should take care of the .?123 character injection I was seeing. Not sure how it got in there after the rebase... https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... File ui/keyboard/resources/webui/keysets.html (right): https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... ui/keyboard/resources/webui/keysets.html:22: <kb-key class="left-shift padded-left-special dark" toKeyset="lower" char="Shift">shift</kb-key><kb-key>Z</kb-key> On 2013/06/03 19:48:49, bshe wrote: > Looks like the char here should be "Invalid". Otherwise, it will send "Shift" > characters. This is probably result from rebase. Done. https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... ui/keyboard/resources/webui/keysets.html:28: <kb-key class="symbol dark" toKeyset="symbol">?123</kb-key> On 2013/06/03 19:48:49, bshe wrote: > Need to add char="Invalid" here too. Done. https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... ui/keyboard/resources/webui/keysets.html:51: <kb-key class="left-shift padded-left-special dark" toKeyset="upper" char="Shift">shift</kb-key><kb-key>z</kb-key> On 2013/06/03 19:48:49, bshe wrote: > Same here. s/Shift/Invalid Done. https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... ui/keyboard/resources/webui/keysets.html:57: <kb-key class="symbol dark" toKeyset="symbol">?123</kb-key> On 2013/06/03 19:48:49, bshe wrote: > Add char="Invalid" Done. https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... ui/keyboard/resources/webui/keysets.html:78: <kb-key class="left-more padded-left-special dark" toKeyset="more">more</kb-key> On 2013/06/03 19:48:49, bshe wrote: > add char="Invalid" Done. https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... ui/keyboard/resources/webui/keysets.html:84: <kb-key class="symbol dark" toKeyset="lower">abc</kb-key> On 2013/06/03 19:48:49, bshe wrote: > add char="Invalid" Done. https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... ui/keyboard/resources/webui/keysets.html:106: <kb-key class="left-more padded-left-special dark" toKeyset="symbol">?123</kb-key><kb-key></kb-key> On 2013/06/03 19:48:49, bshe wrote: > add char="Invalid" Done. https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... ui/keyboard/resources/webui/keysets.html:112: <kb-key class="symbol dark" toKeyset="lower">abc</kb-key> On 2013/06/03 19:48:49, bshe wrote: > add char="Invalid" Done.
On 2013/06/03 20:26:42, SteveT wrote: > Thanks! I think that should take care of the .?123 character injection I was > seeing. Not sure how it got in there after the rebase... > > https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... > File ui/keyboard/resources/webui/keysets.html (right): > > https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... > ui/keyboard/resources/webui/keysets.html:22: <kb-key class="left-shift > padded-left-special dark" toKeyset="lower" > char="Shift">shift</kb-key><kb-key>Z</kb-key> > On 2013/06/03 19:48:49, bshe wrote: > > Looks like the char here should be "Invalid". Otherwise, it will send "Shift" > > characters. This is probably result from rebase. > > Done. > > https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... > ui/keyboard/resources/webui/keysets.html:28: <kb-key class="symbol dark" > toKeyset="symbol">?123</kb-key> > On 2013/06/03 19:48:49, bshe wrote: > > Need to add char="Invalid" here too. > > Done. > > https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... > ui/keyboard/resources/webui/keysets.html:51: <kb-key class="left-shift > padded-left-special dark" toKeyset="upper" > char="Shift">shift</kb-key><kb-key>z</kb-key> > On 2013/06/03 19:48:49, bshe wrote: > > Same here. s/Shift/Invalid > > Done. > > https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... > ui/keyboard/resources/webui/keysets.html:57: <kb-key class="symbol dark" > toKeyset="symbol">?123</kb-key> > On 2013/06/03 19:48:49, bshe wrote: > > Add char="Invalid" > > Done. > > https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... > ui/keyboard/resources/webui/keysets.html:78: <kb-key class="left-more > padded-left-special dark" toKeyset="more">more</kb-key> > On 2013/06/03 19:48:49, bshe wrote: > > add char="Invalid" > > Done. > > https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... > ui/keyboard/resources/webui/keysets.html:84: <kb-key class="symbol dark" > toKeyset="lower">abc</kb-key> > On 2013/06/03 19:48:49, bshe wrote: > > add char="Invalid" > > Done. > > https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... > ui/keyboard/resources/webui/keysets.html:106: <kb-key class="left-more > padded-left-special dark" toKeyset="symbol">?123</kb-key><kb-key></kb-key> > On 2013/06/03 19:48:49, bshe wrote: > > add char="Invalid" > > Done. > > https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... > ui/keyboard/resources/webui/keysets.html:112: <kb-key class="symbol dark" > toKeyset="lower">abc</kb-key> > On 2013/06/03 19:48:49, bshe wrote: > > add char="Invalid" > > Done. Your patchset 2 seems included diffs from my CL. You probably wants to git cl upload LocalBranch?
On 2013/06/03 20:48:48, bshe wrote: > On 2013/06/03 20:26:42, SteveT wrote: > > Thanks! I think that should take care of the .?123 character injection I was > > seeing. Not sure how it got in there after the rebase... > > > > > https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... > > File ui/keyboard/resources/webui/keysets.html (right): > > > > > https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... > > ui/keyboard/resources/webui/keysets.html:22: <kb-key class="left-shift > > padded-left-special dark" toKeyset="lower" > > char="Shift">shift</kb-key><kb-key>Z</kb-key> > > On 2013/06/03 19:48:49, bshe wrote: > > > Looks like the char here should be "Invalid". Otherwise, it will send > "Shift" > > > characters. This is probably result from rebase. > > > > Done. > > > > > https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... > > ui/keyboard/resources/webui/keysets.html:28: <kb-key class="symbol dark" > > toKeyset="symbol">?123</kb-key> > > On 2013/06/03 19:48:49, bshe wrote: > > > Need to add char="Invalid" here too. > > > > Done. > > > > > https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... > > ui/keyboard/resources/webui/keysets.html:51: <kb-key class="left-shift > > padded-left-special dark" toKeyset="upper" > > char="Shift">shift</kb-key><kb-key>z</kb-key> > > On 2013/06/03 19:48:49, bshe wrote: > > > Same here. s/Shift/Invalid > > > > Done. > > > > > https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... > > ui/keyboard/resources/webui/keysets.html:57: <kb-key class="symbol dark" > > toKeyset="symbol">?123</kb-key> > > On 2013/06/03 19:48:49, bshe wrote: > > > Add char="Invalid" > > > > Done. > > > > > https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... > > ui/keyboard/resources/webui/keysets.html:78: <kb-key class="left-more > > padded-left-special dark" toKeyset="more">more</kb-key> > > On 2013/06/03 19:48:49, bshe wrote: > > > add char="Invalid" > > > > Done. > > > > > https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... > > ui/keyboard/resources/webui/keysets.html:84: <kb-key class="symbol dark" > > toKeyset="lower">abc</kb-key> > > On 2013/06/03 19:48:49, bshe wrote: > > > add char="Invalid" > > > > Done. > > > > > https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... > > ui/keyboard/resources/webui/keysets.html:106: <kb-key class="left-more > > padded-left-special dark" toKeyset="symbol">?123</kb-key><kb-key></kb-key> > > On 2013/06/03 19:48:49, bshe wrote: > > > add char="Invalid" > > > > Done. > > > > > https://codereview.chromium.org/16336002/diff/1/ui/keyboard/resources/webui/k... > > ui/keyboard/resources/webui/keysets.html:112: <kb-key class="symbol dark" > > toKeyset="lower">abc</kb-key> > > On 2013/06/03 19:48:49, bshe wrote: > > > add char="Invalid" > > > > Done. > > Your patchset 2 seems included diffs from my CL. You probably wants to git cl > upload LocalBranch? Oh shoot, yeah. I had to run a special command to exclude your CL. Doing that now.
R+Bryan so he can see this.
Okay, I've rebased on top of Biao's patch. PTAL for realz. Note that this is just to get the keyboard into a working state for testing. I'll follow up with another patch later to (1) complete the keyboard layout and (2) give it the v1 style.
When I said Biao's patch, I meant TOT. i.e. That includes Bryan's special character fix (but we're not really using it yet)
On 2013/06/21 14:06:04, SteveT wrote: > When I said Biao's patch, I meant TOT. i.e. That includes Bryan's special > character fix (but we're not really using it yet) lgtm
Thanks. Bryan - when you have a minute, PTAL for OWNERS approval.
lgtm
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/stevet@chromium.org/16336002/13001
Retried try job too often on win_rel for step(s) sync_integration_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/stevet@chromium.org/16336002/13001
Failed to apply patch for ui/keyboard/resources/elements/kb-row.html:
While running patch -p1 --forward --force --no-backup-if-mismatch;
patching file ui/keyboard/resources/elements/kb-row.html
Hunk #1 FAILED at 1.
1 out of 1 hunk FAILED -- saving rejects to file
ui/keyboard/resources/elements/kb-row.html.rej
Patch: ui/keyboard/resources/elements/kb-row.html
Index: ui/keyboard/resources/elements/kb-row.html
diff --git a/ui/keyboard/resources/elements/kb-row.html
b/ui/keyboard/resources/elements/kb-row.html
index
21525b8330e28f8a9f9e30a6155c748fb0e0f88f..40751fd2d13e121bd040ce6746c625232acbf2b3
100644
--- a/ui/keyboard/resources/elements/kb-row.html
+++ b/ui/keyboard/resources/elements/kb-row.html
@@ -1,28 +1,28 @@
-<!--
- -- Copyright (c) 2013 The Chromium Authors. All rights reserved.
- -- Use of this source code is governed by a BSD-style license that can be
- -- found in the LICENSE file.
- -->
-
-<element name="kb-row">
- <template>
- <style>
- @host {
- * {
- -webkit-box-flex: 1;
- display: -webkit-box;
- text-align: center;
- margin-right: 5px;
- margin-top: 5px;
- }
- }
- content::-webkit-distributed(kb-key) {
- -webkit-flex: 1 auto;
- }
- </style>
- <content select="*"></content>
- </template>
- <script>
- Polymer.register(this);
- </script>
-</element>
+<!--
+ -- Copyright (c) 2013 The Chromium Authors. All rights reserved.
+ -- Use of this source code is governed by a BSD-style license that can be
+ -- found in the LICENSE file.
+ -->
+
+<element name="kb-row">
+ <template>
+ <style>
+ @host {
+ * {
+ -webkit-box-flex: 1;
+ display: -webkit-box;
+ text-align: center;
+ margin-right: 5px;
+ margin-top: 7px;
+ }
+ }
+ content::-webkit-distributed(kb-key) {
+ -webkit-flex: 1 auto;
+ }
+ </style>
+ <content select="*"></content>
+ </template>
+ <script>
+ Polymer.register(this);
+ </script>
+</element>
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/stevet@chromium.org/16336002/13001
Failed to apply patch for ui/keyboard/resources/elements/kb-row.html:
While running patch -p1 --forward --force --no-backup-if-mismatch;
patching file ui/keyboard/resources/elements/kb-row.html
Hunk #1 FAILED at 1.
1 out of 1 hunk FAILED -- saving rejects to file
ui/keyboard/resources/elements/kb-row.html.rej
Patch: ui/keyboard/resources/elements/kb-row.html
Index: ui/keyboard/resources/elements/kb-row.html
diff --git a/ui/keyboard/resources/elements/kb-row.html
b/ui/keyboard/resources/elements/kb-row.html
index
21525b8330e28f8a9f9e30a6155c748fb0e0f88f..40751fd2d13e121bd040ce6746c625232acbf2b3
100644
--- a/ui/keyboard/resources/elements/kb-row.html
+++ b/ui/keyboard/resources/elements/kb-row.html
@@ -1,28 +1,28 @@
-<!--
- -- Copyright (c) 2013 The Chromium Authors. All rights reserved.
- -- Use of this source code is governed by a BSD-style license that can be
- -- found in the LICENSE file.
- -->
-
-<element name="kb-row">
- <template>
- <style>
- @host {
- * {
- -webkit-box-flex: 1;
- display: -webkit-box;
- text-align: center;
- margin-right: 5px;
- margin-top: 5px;
- }
- }
- content::-webkit-distributed(kb-key) {
- -webkit-flex: 1 auto;
- }
- </style>
- <content select="*"></content>
- </template>
- <script>
- Polymer.register(this);
- </script>
-</element>
+<!--
+ -- Copyright (c) 2013 The Chromium Authors. All rights reserved.
+ -- Use of this source code is governed by a BSD-style license that can be
+ -- found in the LICENSE file.
+ -->
+
+<element name="kb-row">
+ <template>
+ <style>
+ @host {
+ * {
+ -webkit-box-flex: 1;
+ display: -webkit-box;
+ text-align: center;
+ margin-right: 5px;
+ margin-top: 7px;
+ }
+ }
+ content::-webkit-distributed(kb-key) {
+ -webkit-flex: 1 auto;
+ }
+ </style>
+ <content select="*"></content>
+ </template>
+ <script>
+ Polymer.register(this);
+ </script>
+</element>
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/stevet@chromium.org/16336002/36001
Message was sent while issue was closed.
Change committed as 207982 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
