Closed
Bug 1177564
Opened 9 years ago
Closed 9 years ago
[Rule View] Tweak add rule and toggle pseudo classes button
Categories
(DevTools :: Inspector, defect)
DevTools
Inspector
Tracking
(firefox41 fixed)
RESOLVED
FIXED
Firefox 41
Tracking | Status | |
---|---|---|
firefox41 | --- | fixed |
People
(Reporter: gl, Assigned: gl)
References
Details
Attachments
(1 file, 2 obsolete files)
1.23 KB,
patch
|
gl
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → gabriel.luong
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•9 years ago
|
||
Attachment #8626502 -
Flags: review?(bgrinstead)
Assignee | ||
Comment 2•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0ecae37b6831
Comment 3•9 years ago
|
||
Comment on attachment 8626502 [details] [diff] [review] 1177564.patch Review of attachment 8626502 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/themes/shared/devtools/ruleview.css @@ +295,5 @@ > +} > + > +#ruleview-add-rule-button, > +#pseudo-class-panel-toggle, > +#pseudo-class-panel-toggle[hover], I think you meant :hover here.
Comment 4•9 years ago
|
||
Comment on attachment 8626502 [details] [diff] [review] 1177564.patch Review of attachment 8626502 [details] [diff] [review]: ----------------------------------------------------------------- I actually wonder if we should be adding a generic attribute on the button elements and control this in toolbars.inc.css. We can do that separately though. r=me with Tim's update ::: browser/themes/shared/devtools/ruleview.css @@ +268,5 @@ > .ruleview-selectorhighlighter.highlighted { > background-position: -16px 0; > } > > +#ruleview-add-rule-button, Can you move this rule down next to the :hover rule that sets opacity: 1? Also, can you add a comment above the rule explaining that these buttons are using opacity instead of background color to indicate the state?
Attachment #8626502 -
Flags: review?(bgrinstead)
Assignee | ||
Comment 5•9 years ago
|
||
Attachment #8626502 -
Attachment is obsolete: true
Attachment #8626691 -
Flags: review+
Assignee | ||
Comment 6•9 years ago
|
||
Attachment #8626691 -
Attachment is obsolete: true
Attachment #8626693 -
Flags: review+
Comment 8•9 years ago
|
||
Comment on attachment 8626693 [details] [diff] [review] 1177564.patch Review of attachment 8626693 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/themes/shared/devtools/ruleview.css @@ +291,5 @@ > +#pseudo-class-panel-toggle { > + opacity: 0.8; > +} > + > +#ruleview-add-rule-button:hover, I've just noticed locally that this needs :not([disabled]), otherwise the disabled state will have an hover effect which is unwanted.
Assignee | ||
Comment 9•9 years ago
|
||
(In reply to Tim Nguyen [:ntim] from comment #8) > Comment on attachment 8626693 [details] [diff] [review] > 1177564.patch > > Review of attachment 8626693 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: browser/themes/shared/devtools/ruleview.css > @@ +291,5 @@ > > +#pseudo-class-panel-toggle { > > + opacity: 0.8; > > +} > > + > > +#ruleview-add-rule-button:hover, > > I've just noticed locally that this needs :not([disabled]), otherwise the > disabled state will have an hover effect which is unwanted. Filed Bug 1178193
Comment 10•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/e1196ef8dcfa
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•