Closed
Bug 1373339
Opened 7 years ago
Closed 7 years ago
Add a button to toggle the CSS shapes highlighter
Categories
(DevTools :: Inspector, enhancement, P3)
DevTools
Inspector
Tracking
(firefox56 verified, firefox62 verified)
VERIFIED
FIXED
Firefox 56
People
(Reporter: mpark, Assigned: mpark)
References
(Blocks 1 open bug)
Details
(Keywords: dev-doc-needed)
Attachments
(1 file)
There should be a button to toggle the CSS shapes highlighter created in bug 1282716 on and off. This button should be in the rules view, next to the clip-path/shape-outside definition, like the grid highlighter toggle, and should be behind a preference and not visible by default.
Comment 1•7 years ago
|
||
If I understand correctly, the highlighter created in bug 1282716 doesn't have any user-visible UI yet, right? Sebastian
Status: NEW → ASSIGNED
Keywords: dev-doc-needed
Updated•7 years ago
|
Flags: needinfo?(mpark)
Assignee | ||
Comment 2•7 years ago
|
||
Yes, that's right. Currently the only way to activate the highlighter is by selecting a node with clip-path in the inspector, and executing the following code in the browser toolbox's console: /* -sp-context:browser */ var t = [...gDevTools._toolboxes][0][1]; var i = t.getPanel("inspector"); t.highlighterUtils.getHighlighterByType("ShapesHighlighter").then(h => { h.show(i.selection.nodeFront, {mode: "cssClipPath"}).then(() => { // highlighter is now shown }, e => console.error("could not show", e)); }, e => console.error("could not get highlighter", e));
Flags: needinfo?(mpark)
Updated•7 years ago
|
Priority: -- → P3
Comment 3•7 years ago
|
||
I see. Thank you for the detailed info how to turn the highlighter on! Sebastian
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Updated•7 years ago
|
Attachment #8879630 -
Flags: review?(zer0) → review?(gl)
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8879630 [details] Bug 1373339 - Add a button in the rules view to toggle the CSS shapes highlighter. https://reviewboard.mozilla.org/r/150978/#review164626 ::: commit-message-55b91:1 (Diff revision 2) > +Bug 1373339 - Add a button in the rules view to toggle the CSS shapes highlighter. r=zer0 s/r=zer0/r=gl ::: devtools/client/inspector/rules/test/browser_rules_shapes-toggle_01.js:25 (Diff revision 2) > + > +const HIGHLIGHTER_TYPE = "ShapesHighlighter"; > +const CSS_SHAPES_ENABLED_PREF = "devtools.inspector.shapesHighlighter.enabled"; > + > +add_task(function* () { > + SpecialPowers.setBoolPref(CSS_SHAPES_ENABLED_PREF, true); Instead of doing this in every shapes test file. We should set this in rules/test/head.js See http://searchfox.org/mozilla-central/source/devtools/client/inspector/grids/test/head.js#19 ::: devtools/client/inspector/rules/views/text-property-editor.js:364 (Diff revision 2) > filterSwatchClass: SHARED_SWATCH_CLASS + " " + FILTER_SWATCH_CLASS, > gridClass: "ruleview-grid", > defaultColorType: !propDirty, > urlClass: "theme-link", > - baseURI: this.sheetHref > + baseURI: this.sheetHref, > + shapeClass: "ruleview-shape" Move this after gridClass ::: devtools/client/inspector/shared/highlighters-overlay.js:42 (Diff revision 2) > this.selectorHighlighterShown = null; > + // NodeFront of the shape that is highlighted > + this.shapesHighlighterShown = null; > // Saved state to be restore on page navigation. > this.state = { > // Only the grid highlighter state is saved at the moment. We can remove this. ::: devtools/client/inspector/shared/highlighters-overlay.js:323 (Diff revision 2) > this.geometryEditorHighlighterShown = null; > }), > > /** > * Restore the saved highlighter states. > - * > + * @param {String} name the name of the highlighter to be restored Add a new line to separate the jsdoc and @param ::: devtools/client/inspector/shared/highlighters-overlay.js:324 (Diff revision 2) > }), > > /** > * Restore the saved highlighter states. > - * > + * @param {String} name the name of the highlighter to be restored > + * @param {String} selector the selector of the node that was previously highlighted Put the parameter description on a new line amd capitalize ::: devtools/client/inspector/shared/highlighters-overlay.js:459 (Diff revision 2) > _isRuleViewDisplayGrid: function (node) { > return this.isRuleView && node.classList.contains("ruleview-grid"); > }, > > /** > + * Is the current clicked node have the shapes highlighter toggle in the s/clicked node have the shapes/clicked node the shapes ::: devtools/client/inspector/shared/highlighters-overlay.js:460 (Diff revision 2) > return this.isRuleView && node.classList.contains("ruleview-grid"); > }, > > /** > + * Is the current clicked node have the shapes highlighter toggle in the > + * rule-view s/rule-view/rule-view. ::: devtools/client/inspector/shared/highlighters-overlay.js:556 (Diff revision 2) > this._lastHovered = null; > this._hideHoveredHighlighter(); > }, > > /** > * Handler function for "markupmutation" events. Hides the grid highlighter if the grid Update the JSDoc to include the shapes highlighter ::: devtools/client/inspector/shared/highlighters-overlay.js:600 (Diff revision 2) > * Restore saved highlighter state after navigate. > */ > onNavigate: Task.async(function* () { > try { > - yield this.restoreState(); > + yield this.restoreState("grid", this.state.grid, > + this.showGridHighlighter.bind(this)); Bind these functions on the class initialization ::: devtools/client/inspector/shared/highlighters-overlay.js:614 (Diff revision 2) > * Clear saved highlighter shown properties on will-navigate. > */ > onWillNavigate: function () { > this.geometryEditorHighlighterShown = null; > this.gridHighlighterShown = null; > + this.shapesHighlighterShown = null; Move this after this.selectorHighlighterShown to keep it kinda alphabetically ordered. ::: devtools/client/inspector/shared/highlighters-overlay.js:649 (Diff revision 2) > this.supportsHighlighters = null; > this.state = null; > > this.geometryEditorHighlighterShown = null; > this.gridHighlighterShown = null; > + this.shapesHighlighterShown = null; Same as above. ::: devtools/client/shared/output-parser.js:16 (Diff revision 2) > const { > ANGLE_TAKING_FUNCTIONS, > BEZIER_KEYWORDS, > COLOR_TAKING_FUNCTIONS, > - CSS_TYPES > + CSS_TYPES, > + BASIC_SHAPE_FUNCTIONS Move this after ANGLE_TAKING_FUNCTIONS ::: devtools/client/shared/output-parser.js:81 (Diff revision 2) > parseCssProperty: function (name, value, options = {}) { > options = this._mergeOptions(options); > > options.expectCubicBezier = this.supportsType(name, CSS_TYPES.TIMING_FUNCTION); > options.expectDisplay = name === "display"; > + options.expectShape = name === "clip-path" || name === "shape-outside"; Move this after options.expectFilter ::: devtools/client/shared/output-parser.js:202 (Diff revision 2) > this._appendCubicBezier(functionText, options); > } else if (colorOK() && > colorUtils.isValidCSSColor(functionText, this.cssColor4)) { > this._appendColor(functionText, options); > + } else if (Services.prefs.getBoolPref(CSS_SHAPES_ENABLED_PREF) && > + options.expectShape && I would reorder the options.expectShape to be first to avoid a call to getBoolPref if it didn't expect it. ::: devtools/client/shared/output-parser.js:721 (Diff revision 2) > * - filterSwatch: false // A special case for parsing a > * // "filter" property, causing the > * // parser to skip the call to > * // _wrapFilter. Used only for > * // previewing with the filter swatch. > * - gridClass: "" // The class to use for the grid icon. Add shapeClass jsdoc ::: devtools/client/shared/output-parser.js:743 (Diff revision 2) > filterSwatch: false, > gridClass: "", > supportsColor: false, > urlClass: "", > baseURI: undefined, > + shapeClass: "", Move this after gridClass
Attachment #8879630 -
Flags: review?(gl) → review+
Comment 7•7 years ago
|
||
We should also add unit test for !important
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Comment 9•7 years ago
|
||
Autoland can't push this until the review requirements for MozReview are met. http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview/autoland.html#landing-commits
Keywords: checkin-needed
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 13•7 years ago
|
||
Pushed by gabriel.luong@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/d425977fef75 Add a button in the rules view to toggle the CSS shapes highlighter. r=gl
Comment 14•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d425977fef75
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Comment 15•6 years ago
|
||
Hi everyone, i have retested this issue on Windows 10 , Windows 7 , Mac OSx and Linux using the latest version of nightly 62.0a1 (2018-05-28) and i can Confirm it as Fixed. I will mark it accordingly.
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•