Clochix Reporter
	Description • 13 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.

	Panos Astithas (he/him) [:past] (please ni?)
	Comment 1 • 13 years ago

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

	Paul Rouget [:paul]
	Comment 2 • 13 years ago

This should work for the style Rule View as well.

	Cedric Vivier [:cedricv]
	Comment 4 • 13 years ago

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.

	Cedric Vivier [:cedricv]
	Comment 5 • 13 years ago

(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...

	Rob Campbell [:rc] (:robcee)
	Comment 6 • 13 years ago

I will get to this review tomorrow. Double-plus-promise!

	Rob Campbell [:rc] (:robcee)
	Comment 7 • 13 years ago

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.

	Sebastian Zartner [:sebo]
	Comment 8 • 8 years ago

Cedric wasn't active on BMO for the last few years, so I assume it's save to remove him as assignee.

Sebastian