Add key bindings for CSS value adjustment and conversion in the Style Editor

NEW
Unassigned

Status

P3
enhancement
7 years ago
5 months ago

People

(Reporter: clochix, Unassigned)

Tracking

(Depends on: 1 bug, Blocks: 1 bug)

Trunk
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
Firebug has some nice shortcuts in Style Editor when editing stylesheets. You can easily increment and decrement values with the arrow keys, with misc steps (.1, 1 or 10). See http://getfirebug.com/wiki/index.php/Keyboard_and_Mouse_Shortcuts#CSS_Editing

Would be great to have similar shortcuts in Style Editor.

Updated

7 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
Isn't this already implemented in the add-on version of the Style Editor? See for instance this commit:

https://github.com/neonux/StyleEditor/commit/5fa4ff93a1cdf131ab3e849847ca80b2fb8127fd

Comment 2

7 years ago
This should work for the style Rule View as well.
Duplicate of this bug: 687708
Blocks: 717625
Created attachment 588049 [details] [diff] [review]
Key bindings for CSS value adjustment and conversion

Alt-Up|Down to increment/decrement the CSS value at the cursor position.
When the caret is on a color value, the increment/decrement is done in the HSL colorspace for lighter/darker.

Alt-PageUp|PageDown doubles or halves a CSS value.

Alt-Left|Right to convert the CSS value at the cursor position in a compatible unit (eg. px <=> cm - or - white <=> #fff <=> rgb(255,255,255) <=> ...)

Alt-Shift-Left|Right on a CSS color adjust its hue.
Assignee: nobody → cedricv
Status: NEW → ASSIGNED
Attachment #588049 - Flags: review?(rcampbell)
Summary: Add shortcuts in style editor to increment / decrement values → Add key bindings for CSS value adjustment and conversion in the Style Editor
(In reply to Cedric Vivier [cedricv] from comment #4)
> Alt-Left|Right to convert the CSS value at the cursor position in a
> compatible unit (eg. px <=> cm - or - white <=> #fff <=> rgb(255,255,255)
> <=> ...)

Alt-Left|Right on an enumeration, cycles between available values, eg. on a border-style, it cycles between 'solid', 'dotted', 'dashed' and so on...
I will get to this review tomorrow. Double-plus-promise!
Comment on attachment 588049 [details] [diff] [review]
Key bindings for CSS value adjustment and conversion

@@ -1110,16 +1111,101 @@ StyleEditor.prototype = {
     bindings.push({
       action: "StyleEditor.openInfoForToken",
       code: Ci.nsIDOMKeyEvent.DOM_VK_F1,
       callback: function getTokenInfo() {
         this.openInfoForTokenAtCursor();
       }.bind(this)
     });
 
+    bindings.push({
+      action: "StyleEditor.incrementValue",
+      code: Ci.nsIDOMKeyEvent.DOM_VK_UP,
+      alt: true,
+      callback: function incrementValue() {
+        this.incrementValueAtCursorBy(1);
+      }.bind(this)
+    });

This won't apply cleanly. Looks like it's based on top of your help key patch. Please update the bug dependencies to reflect that.

...
this.getTokenAtCursor(); not defined in this patch. Implemented in another bug (maybe same as above, I guess you'll need it for looking up help)?

in StyleValue.jsm:

+const CONVERSION_TABLE = {
+  "length": {  // canonical unit is 'in'
+    "in": 1,
+    "px": 96,
+    "pt": 72,
+    "pc": 6,
+    "cm": 2.54,
+    "mm": 254
+  },

Do we really not have a platformy way to convert CSS units? I found https://developer.mozilla.org/en/CSS/length which appears to be pretty canonical reference and the units check out.

Maybe include a link to your reference in a comment here? If the platform ever changes to device independence or adds more unit types, we'll have to update and add.

Also found nsColorNameList.h used in nsColors.cpp. It has some conversion functions but may not be strictly applicable. (Thanks to JohanC for mxr-ing that!)

+  "angle": {   // canonical unit is 'turn'
+    "turn": 1,
+    "deg": 360,
+    "grad": 400
+  },

no rads?

+const COLOR_TABLE = {
+  "transparent": [0, 0, 0, 0.0],
+  "black": [0, 0, 0, 1.0],
+  "silver": [192, 192, 192, 1.0],
...

these'll need a reference comment as well (for the others too). Where'd you get your list?

You should probably provide references for all of these tables in comments.

I'm a little concerned about how we're planning to keep all of these up-to-date.

+const ENUMERATION_TABLE = {
+  "border-style": ["dashed", "dotted", "double", "groove", "inset", "outset",
+                   "ridge", "solid"],

and sources for these as well. Looks like border-style doesn't include "hidden" and "none".

Will these enumerations work for border-x-style where x is a position? I haven't gotten to the chunk that may or may not do translations for different types of property.

+  "font-weight": ["bold", "bolder", "lighter"],

what about numerical font-weights? How will they be converted?

+  "text-decoration": ["underline", "overline", "line-through"],

what? no "blink"? :)

in StyleValue():

+  if (text[0] == "#") {
+    let h = text.match(/^#([a-fA-F0-9])([a-fA-F0-9])([a-fA-F0-9])$/) ||
+            text.match(/^#([a-fA-F0-9]{2})([a-fA-F0-9]{2})([a-fA-F0-9]{2})$/);

maybe rename that variable to "hex" to make it a little clearer that you're matching hex values.

+    let rgba = text.match(/^rgb\(([0-9]+),([0-9]+),([0-9]+)\)$/) ||
+               text.match(/^rgba\(([0-9]+),([0-9]+),([0-9]+),([0-9.]+)\)$/);

I'm pretty sure you could minimize this into a one-liner, but it's probably not worth it.

will this handle whitespace after the ',' or between values? Doesn't look like it here, but you may be sanitizing the token on entry. The trim() at the beginning of the function only handles leading and trailing whitespace, not internal.

also, looking at the second line, it looks like you could match a value like, ..001.22.....2... which, obviously shouldn't be in there, but I'd be curious to see what would happen.

same for hsla's regex.

I'm not sure any of that matters in the context of this review, but it might be worth checking for odd values in aToken. Or weird values in the CSS Editor itself to see how this all copes with them.

r- because we're not being polite anymore. ;)

Make sure to update the dependencies.
Attachment #588049 - Flags: review?(rcampbell) → review-
Depends on: 687707
Priority: -- → P3
Cedric wasn't active on BMO for the last few years, so I assume it's save to remove him as assignee.

Sebastian
Assignee: cedricv → nobody
Status: ASSIGNED → NEW

Updated

5 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.