Closed Bug 1259559 Opened 9 years ago Closed 9 years ago

Units cycling with shift+click should persists and change the value in Style editor

Categories

(DevTools :: Inspector: Rules, defect, P2)

Product:
DevTools
Component:
Inspector: Rules
Version:
48 Branch
Type:
defect

Tracking

(firefox48 verified)

Status:
VERIFIED FIXED
Milestone:
Firefox 48
Tracking Flags:
Tracking Status
firefox48 --- verified

People

(Reporter: nchevobbe, Assigned: nchevobbe)

References

Details

(Whiteboard: [btpp-fix-later])

Attachments

(2 files)

angle.html
940 bytes, text/html
Details
MozReview Request: Bug 1259559 - Units cycling with shift+click persists value in Style editor. r=miker
58 bytes, text/x-review-board-request
miker
: review+
Details
Attached file angle.html —
For color and angle values, we can cycle through the units ( rgb -> hex -> hsl -> name , deg -> rad -> grad -> turn ) with shift + click. Those unit changes don't persist though. STR : 1. Open the attached file 2. Open inspector 3. Select the <body> node 4. shift + click on the `rgba(183,222,237,1)` color swatch to the hex value (`#b7deed`) 5. select the <html> node 6. select the <body> node Expected result : The first color of the gradient should be `#b7deed`. Actual result : The first color of the gradient is `rgba(183,222,237,1)`, the original value.
Has STR: --- → yes
Component: Developer Tools → Developer Tools: CSS Rules Inspector
Version: 47 Branch → 48 Branch
Assignee: nobody → chevobbe.nicolas
Triaging (filter on CLIMBING SHOES).
Priority: -- → P2
Whiteboard: [btpp-fix-later]
So I thought a little more about this one and there is something that should be taken care of. In the option panel, you can chose how colours should be displayed. It can be "authored", i.e. like it is declared by the user in the rule, or it can be one of hex, hsl, rgba, name. Maybe we could only persist value through shift+click if the pref is set to "authored". But it can be confusing to both persist and not, only depending of the state of a preference. What do you think about it ?
Flags: needinfo?(ntim.bugs)
Flags: needinfo?(hholmes)
I think we should always persist, whether it was authored or not. I think that the changing would be more confusing than anything else, like you've already mentioned.
Flags: needinfo?(hholmes)
We should persist in any case. Editing the unit manually without shift click does persist, and we should do the same with shift click
Flags: needinfo?(ntim.bugs)
Add an 'unit-change' event fired when shift+click on color and angle swatches. Add a listener on this event in text-property-editor.js to call the same function that's called when tooltip edit is commited to persist the new unit. Edit some tests to adapt to this new behaviour and create some tests to make sure the value obtained via shift+click are actually persisted. Review commit: https://reviewboard.mozilla.org/r/43039/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/43039/
Attachment #8736013 - Flags: review?(mratcliffe)
Status: NEW → ASSIGNED
Comment on attachment 8736013 [details] MozReview Request: Bug 1259559 - Units cycling with shift+click persists value in Style editor. r=miker Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43039/diff/1-2/
Comment on attachment 8736013 [details] MozReview Request: Bug 1259559 - Units cycling with shift+click persists value in Style editor. r=miker https://reviewboard.mozilla.org/r/43039/#review39733 Looks spot on... r+ assuming a green try. ::: devtools/client/inspector/rules/test/browser_rules_cycle-color.js:18 (Diff revision 2) > } > + span { > + color: blue; > + } > </style> > - Test cycling color types in the rule view! > + <body><span>Test</span>cycling color types in the rule view!</body> Nit: Add a space after the closing span.
Attachment #8736013 - Flags: review?(mratcliffe) → review+
Comment on attachment 8736013 [details] MozReview Request: Bug 1259559 - Units cycling with shift+click persists value in Style editor. r=miker Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43039/diff/2-3/
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1ddcc57f9baf The TRY run is over and i think everything is good Errors known : dt8 on Windows 7 debug : browser_webconsole_bug_632817.js https://bugzilla.mozilla.org/show_bug.cgi?id=1244707 dt3 on Linux debug| dt2 on Linux 64 debug : No bug filed, but appears in another TRY run without my patch ( https://treeherder.mozilla.org/#/jobs?repo=try&revision=7db58284a44ef0cbd4518ae34419a66ed1f82b35&selectedJob=18769152 )
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/6d91363a7eac
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
I have reproduced this bug with nightly 48.0a1 (2016-03-24) on Windows 10, 64 bit! The Bug's fix is now verified on Latest Beta 48.0b6 Build ID 20160706215822 User Agent Mozilla/5.0 (Windows NT 10.0; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0 [bugday-20160713]
Status: RESOLVED → VERIFIED
status-firefox48: fixed → verified
Depends on: 1284852
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: