Bug 1572662 - Update logic to toggle SelectorHighlighter from Rules view r=jdescottes

Depends on D90246

This patch updates the logic to toggle the selector highlighter (icon next to CSS selectors in the Rules view) using the process-agnostic approach introduced in `HighlightersOverlay`.

There are 3 main chunks of logic:

- A) Introduce event delegation in the Rules view (`CssRuleView.handleEvent()`)
- B) Introduce generic handler for highligther events in Rules view (`CssRuleView.handleHighlighterEvent()`)
- C) Toggle the selector highlighter using the `HighlightersOverlay.showHighlighterTypeForNode()`/`HighlightersOverlay.hideHighlighterType()` methods with the `SELECTOR` highlighter type.

### Part A
With Part A, CssRuleView.handleEvent(), we're laying the groundwork to have the Rules view react to events within its DOM tree.

Currently, `HighlightersOverlay` is doing too much. Along with managing highlighters, it acts as an event delegate for the Rules view and Computed view via [HighlightersOverlay.addToView()](https://searchfox.org/mozilla-central/rev/0c97a6410ff018c22e65a0cbe4e5f2ca4581b22e/devtools/client/inspector/shared/highlighters-overlay.js#415-428). This adds [cumbersome logic to check the target](https://searchfox.org/mozilla-central/rev/0c97a6410ff018c22e65a0cbe4e5f2ca4581b22e/devtools/client/inspector/shared/highlighters-overlay.js#1402-1428) of an event in order to know whether to react to it. This isn't wrong per-se, but it concentrates DOM knowledge from a broad part of the Inspector away from where it is generated.

Ideally, `HighlightersOverlay` should only manage highlighters. It should be called from various endpoints without regard to who is calling and in reaction to which events.

The intent is to reuse this `CssRuleView.handleEvent()` for:
- toggling the CSS Transforms Highlighter by reacting to mouseover events from corresponding CSS values
- toggling the Flexbox / Grid / Shapes highlighters by reacting to clicks on corresponding swatch icons next to CSS values

### Part B
Part B, `CssRuleView.handleHighlighterEvent()`, adds a generic highlighter event handler to the Rules view. Checking the event name and highlighter type, the Rules view can update itself in reaction to highlighters triggered from elsewhere in the Inspector.

In this patch, we're using it to toggle the active CSS class name on the selector highlighter icon in response to "highlighter-shown" / "highlighter-hidden" events of the `SELECTOR` highlighter type.

Probably a bit overkill here. But it gets more useful with the Flexbox and Grid Highlighters which have call sites in multiple places with indicators that need to be reconciled:

- Flex/Grid badges in Markup view
- Checkboxes in Layout panel
- Swatch icons for Flex and Grid in Rules view

### Part C
Part C replaces the legacy toggling logic for selector highlighter with the abstract methods in `HighligthersOverlay`, does some slight clean-up, and fixes some bugs in the previous implementation.

All CSS rules matching the same selector will be marked when the selector is active (see inline comment)
When editing a selector, the selector highlighter for another CSS rule will no longer be hidden (see inline comment)

Differential Revision: https://phabricator.services.mozilla.com/D90247
This commit is contained in:
Razvan Caliman 2020-09-18 10:34:08 +00:00
Родитель f36f05b770
Коммит ebfa44cb82
3 изменённых файлов: 245 добавлений и 118 удалений

Просмотреть файл

@ -164,6 +164,11 @@ function CssRuleView(inspector, document, store) {
this.refreshPanel = this.refreshPanel.bind(this);
const doc = this.styleDocument;
// Delegate bulk handling of events happening within the DOM tree of the Rules view
// to this.handleEvent(). Listening on the capture phase of the event bubbling to be
// able to stop event propagation on a case-by-case basis and prevent event target
// ancestor nodes from handling them.
this.styleDocument.addEventListener("click", this, { capture: true });
this.element = doc.getElementById("ruleview-container-focusable");
this.addRuleButton = doc.getElementById("ruleview-add-rule-button");
this.searchField = doc.getElementById("ruleview-searchbox");
@ -181,6 +186,16 @@ function CssRuleView(inspector, document, store) {
this.searchClearButton.hidden = true;
this.onHighlighterShown = data =>
this.handleHighlighterEvent("highlighter-shown", data);
this.onHighlighterHidden = data =>
this.handleHighlighterEvent("highlighter-hidden", data);
this.inspector.highlighters.on("highlighter-shown", this.onHighlighterShown);
this.inspector.highlighters.on(
"highlighter-hidden",
this.onHighlighterHidden
);
this.shortcuts = new KeyShortcuts({ window: this.styleWindow });
this._onShortcut = this._onShortcut.bind(this);
this.shortcuts.on("Escape", event => this._onShortcut("Escape", event));
@ -305,35 +320,7 @@ CssRuleView.prototype = {
},
/**
* Get an instance of SelectorHighlighter (used to highlight nodes that match
* selectors in the rule-view). A new instance is only created the first time
* this function is called. The same instance will then be returned.
*
* @return {Promise} Resolves to the instance of the highlighter.
*/
async getSelectorHighlighter() {
if (!this.inspector) {
return null;
}
if (this.selectorHighlighter) {
return this.selectorHighlighter;
}
try {
const front = this.inspector.inspectorFront;
const h = await front.getHighlighterByType("SelectorHighlighter");
this.selectorHighlighter = h;
return h;
} catch (e) {
// The SelectorHighlighter type could not be created in the
// current target. It could be an older server, or a XUL page.
return null;
}
},
/**
* Highlight/unhighlight all the nodes that match a given set of selectors
* Highlight/unhighlight all the nodes that match a given selector
* inside the document of the current selected node.
* Only one selector can be highlighted at a time, so calling the method a
* second time with a different selector will first unhighlight the previously
@ -341,43 +328,24 @@ CssRuleView.prototype = {
* Calling the method a second time with the same selector will just
* unhighlight the highlighted nodes.
*
* @param {DOMNode} selectorIcon
* The icon that was clicked to toggle the selector. The
* class 'highlighted' will be added when the selector is
* highlighted.
* @param {String} selector
* The selector used to find nodes in the page.
* Elements matching this selector will be highlighted on the page.
*/
async toggleSelectorHighlighter(selectorIcon, selector) {
if (this.lastSelectorIcon) {
this.lastSelectorIcon.classList.remove("highlighted");
}
selectorIcon.classList.remove("highlighted");
const highlighter = await this.getSelectorHighlighter();
if (!highlighter) {
return;
}
await highlighter.hide();
if (selector !== this.highlighters.selectorHighlighterShown) {
this.highlighters.selectorHighlighterShown = selector;
selectorIcon.classList.add("highlighted");
this.lastSelectorIcon = selectorIcon;
const node = this.inspector.selection.nodeFront;
await highlighter.show(node, {
hideInfoBar: true,
hideGuides: true,
selector,
});
this.emit("ruleview-selectorhighlighter-toggled", true);
async toggleSelectorHighlighter(selector) {
if (this.isSelectorHighlighted(selector)) {
await this.inspector.highlighters.hideHighlighterType(
this.inspector.highlighters.TYPES.SELECTOR
);
} else {
this.highlighters.selectorHighlighterShown = null;
this.emit("ruleview-selectorhighlighter-toggled", false);
await this.inspector.highlighters.showHighlighterTypeForNode(
this.inspector.highlighters.TYPES.SELECTOR,
this.inspector.selection.nodeFront,
{
hideInfoBar: true,
hideGuides: true,
selector,
}
);
}
},
@ -393,6 +361,89 @@ CssRuleView.prototype = {
);
},
/**
* Check whether a SelectorHighlighter is active for the given selector text.
*
* @param {String} selector
* @return {Boolean}
*/
isSelectorHighlighted(selector) {
const options = this.inspector.highlighters.getOptionsForActiveHighlighter(
this.inspector.highlighters.TYPES.SELECTOR
);
return options?.selector === selector;
},
/**
* Delegate handler for events happening within the DOM tree of the Rules view.
* Itself delegates to specific handlers by event type.
*
* Use this instead of attaching specific event handlers when:
* - there are many elements with the same event handler (eases memory pressure)
* - you want to avoid having to remove event handlers manually
* - elements are added/removed from the DOM tree arbitrarily over time
*
* @param {MouseEvent|UIEvent} event
*/
handleEvent(event) {
switch (event.type) {
case "click":
this.handleClickEvent(event);
break;
default:
}
},
/**
* Delegate handler for click events happening within the DOM tree of the Rules view.
*
* @param {MouseEvent} event
*/
handleClickEvent(event) {
const target = event.target;
// Handle click on the icon next to a CSS selector.
if (target.classList.contains("js-toggle-selector-highlighter")) {
this.toggleSelectorHighlighter(target.dataset.selector);
// Prevent the click on the element wrapping the CSS rule
// from triggering the prompt to add a new CSS declaration
event.stopPropagation();
}
},
/**
* Delegate handler for highlighter events.
*
* This is the place to observe for highlighter events, check the highlighter type and
* event name, then react to specific events, for example by modifying the DOM.
*
* @param {String} eventName
* Highlighter event name. One of: "highlighter-hidden", "highlighter-shown"
* @param {Object} data
* Object with data associated with the highlighter event.
*/
handleHighlighterEvent(eventName, data) {
switch (data.type) {
// Toggle the "highlighted" CSS class name on selector icons in the Rules view when
// the SelectorHighlighter is shown/hidden for a certain CSS selector.
case this.inspector.highlighters.TYPES.SELECTOR:
if (data?.options?.selector) {
const selector = data?.options?.selector;
const query = `.js-toggle-selector-highlighter[data-selector='${selector}']`;
for (const node of this.styleDocument.querySelectorAll(query)) {
if (eventName == "highlighter-hidden") {
node.classList.remove("highlighted");
}
if (eventName == "highlighter-shown") {
node.classList.add("highlighted");
}
}
}
break;
}
},
/**
* Initializes the content-viewer front and enable the print and color scheme simulation
* if they are supported in the current target.
@ -756,6 +807,7 @@ CssRuleView.prototype = {
// Remove bound listeners
this.shortcuts.destroy();
this.styleDocument.removeEventListener("click", this);
this.element.removeEventListener("copy", this._onCopy);
this.element.removeEventListener("contextmenu", this._onContextMenu);
this.addRuleButton.removeEventListener("click", this._onAddRule);
@ -770,6 +822,14 @@ CssRuleView.prototype = {
this._onTogglePseudoClassPanel
);
this.classToggle.removeEventListener("click", this._onToggleClassPanel);
this.inspector.highlighters.off(
"highlighter-shown",
this.onHighlighterShown
);
this.inspector.highlighters.off(
"highlighter-hidden",
this.onHighlighterHidden
);
this.searchField = null;
this.searchClearButton = null;
@ -1033,8 +1093,6 @@ CssRuleView.prototype = {
* Clear the rule view.
*/
clear: function(clearDom = true) {
this.lastSelectorIcon = null;
if (clearDom) {
this._clearRules();
}

Просмотреть файл

@ -159,6 +159,7 @@ RuleEditor.prototype = {
}
if (this.rule.domRule.type !== CSSRule.KEYFRAME_RULE) {
// FIXME: Avoid having this as a nested async operation. (Bug 1664511)
(async function() {
let selector;
@ -174,26 +175,15 @@ RuleEditor.prototype = {
selector = this.ruleView.inspector.selectionCssSelector;
}
const isHighlighted =
this.ruleView._highlighters &&
this.ruleView.highlighters.selectorHighlighterShown === selector;
const selectorHighlighter = createChild(header, "span", {
const isHighlighted = this.ruleView.isSelectorHighlighted(selector);
// Handling of click events is delegated to CssRuleView.handleEvent()
createChild(header, "span", {
class:
"ruleview-selectorhighlighter" +
"ruleview-selectorhighlighter js-toggle-selector-highlighter" +
(isHighlighted ? " highlighted" : ""),
"data-selector": selector,
title: l10n("rule.selectorHighlighter.tooltip"),
});
selectorHighlighter.addEventListener("click", event => {
this.ruleView.toggleSelectorHighlighter(
selectorHighlighter,
selector
);
// Prevent clicks from focusing the property editor.
event.stopPropagation();
});
this.uniqueSelector = selector;
this.emit("selector-icon-created");
}
.bind(this)()
.catch(error => {
@ -668,6 +658,11 @@ RuleEditor.prototype = {
this.isEditing = true;
// Remove highlighter for the previous selector.
if (this.ruleView.isSelectorHighlighted(this.rule.selectorText)) {
await this.ruleView.toggleSelectorHighlighter(this.rule.selectorText);
}
try {
const response = await this.rule.domRule.modifySelector(element, value);
@ -715,14 +710,6 @@ RuleEditor.prototype = {
// pseudo-element rules and the like.
this.element.parentNode.replaceChild(editor.element, this.element);
// Remove highlight for modified selector
if (ruleView.highlighters.selectorHighlighterShown) {
ruleView.toggleSelectorHighlighter(
ruleView.lastSelectorIcon,
ruleView.highlighters.selectorHighlighterShown
);
}
editor._moveSelectorFocus(direction);
} catch (err) {
this.isEditing = false;

Просмотреть файл

@ -157,8 +157,6 @@ class HighlightersOverlay {
this.geometryEditorHighlighterShown = null;
// Name of the highlighter shown on mouse hover.
this.hoveredHighlighterShown = null;
// Name of the selector highlighter shown.
this.selectorHighlighterShown = null;
// NodeFront of the shape that is highlighted
this.shapesHighlighterShown = null;
@ -205,6 +203,74 @@ class HighlightersOverlay {
EventEmitter.decorate(this);
}
/**
* Optionally run some operations before showing a highligther of a given type.
*
* Depending its type, before showing a new instance of a highlighter, we may do extra
* operations, like hiding another visible highlighter, or preventing the show
* operation, for example due to a duplicate call with the same arguments.
*
* Returns a promise that resovles with a boolean indicating whether to skip showing
* the highlighter with these arguments.
*
* @param {String} type
* Highlighter type to show.
* @param {NodeFront} nodeFront
* Node front of the element to be highlighted.
* @param {Options} options
* Optional object with options to pass to the highlighter.
* @return {Promise}
*/
async _beforeShowHighlighterTypeForNode(type, nodeFront, options) {
// Get the data associated with the visible highlighter of this type, if any.
const {
highlighter: activeHighlighter,
nodeFront: activeNodeFront,
options: activeOptions,
timer: activeTimer,
} = this.getDataForActiveHighlighter(type);
// There isn't an active highlighter of this type. Early return, proceed with showing.
if (!activeHighlighter) {
return false;
}
// Whether conditions are met to skip showing the highlighter (ex: duplicate calls).
let skipShow = false;
// Clear any autohide timer associated with this highlighter type.
// This clears any existing timer for duplicate calls to show() if:
// - called with different options.duration
// - called once with options.duration, then without (see deepEqual() above)
clearTimeout(activeTimer);
switch (type) {
// Hide the visible selector highlighter if called for the same node,
// but with a different selector.
case TYPES.SELECTOR:
if (
nodeFront === activeNodeFront &&
options?.selector !== activeOptions?.selector
) {
await this.hideHighlighterType(TYPES.SELECTOR);
}
break;
// For others, hide the existing highlighter before showing it for a different node.
// Else, if the node is the same and options are the same, skip a duplicate call.
// Duplicate calls to show the highlighter for the same node are allowed
// if the options are different (for example, when scheduling autohide).
default:
if (nodeFront !== activeNodeFront) {
await this.hideHighlighterType(type);
} else if (deepEqual(options, activeOptions)) {
skipShow = true;
}
}
return skipShow;
}
/**
* Get a highlighter instance of the given type for the given node front.
*
@ -239,6 +305,39 @@ class HighlightersOverlay {
return highlighter;
}
/**
* Get an object with data associated with the active highlighter of a given type.
* This data object contains:
* - nodeFront: NodeFront of the highlighted node
* - highlighter: Highlighter instance
* - options: Configuration options passed to the highlighter
* - timer: (Optional) index of timer set with setTimout() to autohide the highlighter
* Returns an empty object if a highlighter of the given type is not active.
*
* @param {String} type
* Highlighter type.
* @return {Object}
*/
getDataForActiveHighlighter(type) {
if (!this._activeHighlighters.has(type)) {
return {};
}
return this._activeHighlighters.get(type);
}
/**
* Get the configuration options of the active highlighter of a given type.
*
* @param {String} type
* Highlighter type.
* @return {Object}
*/
getOptionsForActiveHighlighter(type) {
const { options } = this.getDataForActiveHighlighter(type);
return options;
}
/**
* Get the node front highlighted by a given highlighter type.
*
@ -274,28 +373,14 @@ class HighlightersOverlay {
* @return {Promise}
*/
async showHighlighterTypeForNode(type, nodeFront, options) {
if (this._activeHighlighters.has(type)) {
const {
nodeFront: activeNodeFront,
options: activeOptions,
timer: activeTimer,
} = this._activeHighlighters.get(type);
const skipShow = await this._beforeShowHighlighterTypeForNode(
type,
nodeFront,
options
);
// Hide the existing highlighter before showing it for a different node.
// Else, if the node is the same and options are the same, skip duplicate call.
// Duplicate calls to show the highlighter for the same node are allowed
// if the options are different (for example, when scheduling autohide).
if (nodeFront !== activeNodeFront) {
await this.hideHighlighterType(type);
} else if (deepEqual(options, activeOptions)) {
return;
}
// Clear any autohide timer associated with this highlighter type.
// This clears any existing timer for duplicate calls to show() if:
// - called with different options.duration
// - called once with options.duration, then without (see deepEqual() above)
clearTimeout(activeTimer);
if (skipShow) {
return;
}
const highlighter = await this._getHighlighterTypeForNode(type, nodeFront);
@ -354,9 +439,8 @@ class HighlightersOverlay {
return;
}
const { highlighter, nodeFront, timer } = this._activeHighlighters.get(
type
);
const data = this.getDataForActiveHighlighter(type);
const { highlighter, nodeFront, timer } = data;
// Clear any autohide timer associated with this highlighter type.
clearTimeout(timer);
this._activeHighlighters.delete(type);
@ -367,7 +451,7 @@ class HighlightersOverlay {
if (HIGHLIGHTER_EVENTS[type]?.hidden) {
this.emit(HIGHLIGHTER_EVENTS[type].hidden, nodeFront);
}
this.emit("highlighter-hidden", { type, highlighter, nodeFront });
this.emit("highlighter-hidden", { type, ...data });
}
async canGetParentGridNode() {
@ -1634,7 +1718,6 @@ class HighlightersOverlay {
this.flexboxHighlighterShown = null;
this.geometryEditorHighlighterShown = null;
this.hoveredHighlighterShown = null;
this.selectorHighlighterShown = null;
this.shapesHighlighterShown = null;
}
@ -1732,7 +1815,6 @@ class HighlightersOverlay {
this.flexboxHighlighterShown = null;
this.geometryEditorHighlighterShown = null;
this.hoveredHighlighterShown = null;
this.selectorHighlighterShown = null;
this.shapesHighlighterShown = null;
this.destroyed = true;