985206 - Insert ":" after completing a CSS property name in the style editor

Status: RESOLVED FIXED

(DevTools :: Style Editor, defect)

Description

[Heather Arthur [:harth]]

When you press enter or select an autocomplete suggestion for a CSS property name in the Style Editor, a colon and space should be inserted so that you can go right into typing the property value.

Comment 1

[Heather Arthur [:harth]]

Attachment: Add a ": " after pressing Enter on a CSS property name

This just covers the case of pressing Enter, I didn't attempt to add it in the case where you're using up/down to cycle through suggestions. The patch also fixes a typo: "keyrames" -> "keyframes".

Comment 2

[Girish Sharma [:Optimizer]]

Comment on attachment 8396048 [details] [diff] [review]
Add a ": " after pressing Enter on a CSS property name

I think the right way here would be to add the ": " in the suggestions' label property, so that it gets applied on TAB/ENTER/UP/DOWN, and also user is aware of the fact that autocompleting to this property will also add ": ".

At a later point (in a follow up) we can be more intelligent about these insertions (including !important;) so that we do not add ";" or ": " again, if already present.

Comment 3

[Heather Arthur [:harth]]

(In reply to Girish Sharma [:Optimizer] from comment #2)
> Comment on attachment 8396048 [details] [diff] [review]
> Add a ": " after pressing Enter on a CSS property name
> 
> I think the right way here would be to add the ": " in the suggestions'
> label property, so that it gets applied on TAB/ENTER/UP/DOWN, and also user
> is aware of the fact that autocompleting to this property will also add ": ".

I just tried this out, it doesn't look good.

Comment 4

[Girish Sharma [:Optimizer]]

You mean the repeating : in the suggestions list ?

Then at least can you make it work for all possible ways of insertions ? TAB, UP, Down, etc. ?

Comment 5

[Heather Arthur [:harth]]

Attachment: Add ":" after autocompleting a CSS property name

Yeah, it looked bad when the colon appeared in each item in the popup. I got it so that it will also insert a colon when you press the cycle keys.

Comment 6

[Girish Sharma [:Optimizer]]

Comment on attachment 8396650 [details] [diff] [review]
Add ":" after autocompleting a CSS property name

Review of attachment 8396650 [details] [diff] [review]:
-----------------------------------------------------------------

Everything looks good.

Just about browser_styleeditor_autocomplete.js : I like that its more readable now, but honestly, there is too much duplicate and repeating code. I don't have strong feelings against it though.
Also, a try would be nice :)

Comment 7

[Heather Arthur [:harth]]

Thanks for the feedback! Here's try running: https://tbpl.mozilla.org/?tree=Try&rev=e1bef4d5234d

Comment 8

[Heather Arthur [:harth]]

Try looks good.

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/e472ec4a5280

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/e472ec4a5280