Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Web Inspector: Show grid/flex overlays when in element selection
https://bugs.webkit.org/show_bug.cgi?id=251737
<rdar://problem/105041619>

Reviewed by Patrick Angle.

This allows for developers to more easily visually identify the organization of a node's contents using element select mode (e.g. hovering over a `gap` area will now be more recognizable due to the hatching shown by the flex overlay).

* Source/JavaScriptCore/inspector/protocol/DOM.json:
Move all the configuration options for grid and flex overlays to bespoke protocol objects.

* Source/JavaScriptCore/inspector/scripts/codegen/generator.py:
Ensure that these protocol objects have helper constants for pulling values out of them.

* Source/WebCore/inspector/InspectorOverlay.h:
* Source/WebCore/inspector/InspectorOverlay.cpp:
(WebCore::InspectorOverlay::paint):
(WebCore::InspectorOverlay::getHighlight):
(WebCore::InspectorOverlay::shouldShowOverlay const):
(WebCore::InspectorOverlay::showHighlightGridOverlayForNode): Added.
(WebCore::InspectorOverlay::hideHighlightGridOverlay): Added.
(WebCore::InspectorOverlay::showHighlightFlexOverlayForNode): Added.
(WebCore::InspectorOverlay::hideHighlightFlexOverlay): Added.
In addition to the always-on grid/flex overlay data, have another member variable for a single grid/flex overlay that takes precedence (and is drawn on top) of all other grid/flex overlays.

* Source/WebCore/inspector/agents/InspectorDOMAgent.h:
* Source/WebCore/inspector/agents/InspectorDOMAgent.cpp:
(WebCore::parseRequiredConfigColor): Renamed from `parseConfigColor`.
(WebCore::parseOptionalConfigColor): Added.
(WebCore::InspectorDOMAgent::willDestroyFrontendAndBackend):
(WebCore::InspectorDOMAgent::handleTouchEvent):
(WebCore::InspectorDOMAgent::inspect):
(WebCore::InspectorDOMAgent::highlightMousedOverNode):
(WebCore::InspectorDOMAgent::setSearchingForNode):
(WebCore::InspectorDOMAgent::highlightConfigFromInspectorObject):
(WebCore::InspectorDOMAgent::gridOverlayConfigFromInspectorObject): Added.
(WebCore::InspectorDOMAgent::flexOverlayConfigFromInspectorObject): Added.
(WebCore::InspectorDOMAgent::setInspectModeEnabled):
(WebCore::InspectorDOMAgent::showGridOverlay):
(WebCore::InspectorDOMAgent::showFlexOverlay):
* Source/WebInspectorUI/UserInterface/Models/DOMNode.js:
(WI.DOMNode.get defaultLayoutOverlayColor): Added.
(WI.DOMNode.prototype.showLayoutOverlay):
Handle changes to `DOM.showGridOverlay` and `DOM.showFlexOverlay`.

* Source/WebInspectorUI/UserInterface/Controllers/DOMManager.js:
(WI.DOMManager.prototype.set inspectModeEnabled):
Pass along the current grid/flex overlay configuration when enabling element selection mode.

* Source/WebInspectorUI/UserInterface/Base/Setting.js:
* Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.js:
(WI.SettingsTabContentView.prototype._createElementsSettingsView):
Add some on-by-default `WI.Setting` to control this behavior.

* Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js:

* LayoutTests/inspector/dom/showFlexOverlay.html:
* LayoutTests/inspector/dom/showGridOverlay.html:

Canonical link: https://commits.webkit.org/259989@main
  • Loading branch information
dcrousso committed Feb 8, 2023
1 parent f488ed3 commit f51cad2
Show file tree
Hide file tree
Showing 13 changed files with 294 additions and 65 deletions.
2 changes: 1 addition & 1 deletion LayoutTests/inspector/dom/showFlexOverlay.html
Expand Up @@ -142,7 +142,7 @@

InspectorTest.log("Requesting to show flex overlay for invalid nodeId -1");
await InspectorTest.expectException(async () => {
await DOMAgent.showFlexOverlay(-1, WI.Color.fromString("magenta").toProtocol());
await DOMAgent.showFlexOverlay(-1, { flexColor: WI.Color.fromString("magenta").toProtocol() });
});

await checkFlexOverlayCount(0);
Expand Down
2 changes: 1 addition & 1 deletion LayoutTests/inspector/dom/showGridOverlay.html
Expand Up @@ -142,7 +142,7 @@

InspectorTest.log("Requesting to show grid overlay for invalid nodeId -1");
await InspectorTest.expectException(async () => {
await DOMAgent.showGridOverlay(-1, WI.Color.fromString("magenta").toProtocol());
await DOMAgent.showGridOverlay(-1, { gridColor: WI.Color.fromString("magenta").toProtocol() });
});

await checkGridOverlayCount(0);
Expand Down
38 changes: 29 additions & 9 deletions Source/JavaScriptCore/inspector/protocol/DOM.json
Expand Up @@ -169,6 +169,28 @@
{ "name": "marginColor", "$ref": "RGBAColor", "optional": true, "description": "The margin highlight fill color (default: transparent)." }
]
},
{
"id": "GridOverlayConfig",
"type": "object",
"description": "Configuration data for grid overlays.",
"properties": [
{ "name": "gridColor", "$ref": "RGBAColor", "description": "The primary color to use for the grid overlay." },
{ "name": "showLineNames", "type": "boolean", "optional": true, "description": "Show labels for grid line names. If not specified, the default value is false." },
{ "name": "showLineNumbers", "type": "boolean", "optional": true, "description": "Show labels for grid line numbers. If not specified, the default value is false." },
{ "name": "showExtendedGridLines", "type": "boolean", "optional": true, "description": "Show grid lines that extend beyond the bounds of the grid. If not specified, the default value is false." },
{ "name": "showTrackSizes", "type": "boolean", "optional": true, "description": "Show grid track size information. If not specified, the default value is false." },
{ "name": "showAreaNames", "type": "boolean", "optional": true, "description": "Show labels for grid area names. If not specified, the default value is false." }
]
},
{
"id": "FlexOverlayConfig",
"type": "object",
"description": "Configuration data for flex overlays.",
"properties": [
{ "name": "flexColor", "$ref": "RGBAColor", "description": "The primary color to use for the flex overlay." },
{ "name": "showOrderNumbers", "type": "boolean", "optional": true, "description": "Show labels for flex order. If not specified, the default value is false." }
]
},
{
"id": "Styleable",
"type": "object",
Expand Down Expand Up @@ -425,7 +447,9 @@
"condition": "defined(WTF_PLATFORM_IOS_FAMILY) && WTF_PLATFORM_IOS_FAMILY",
"parameters": [
{ "name": "enabled", "type": "boolean", "description": "True to enable inspection mode, false to disable it." },
{ "name": "highlightConfig", "$ref": "HighlightConfig", "optional": true, "description": "A descriptor for the highlight appearance of hovered-over nodes. May be omitted if <code>enabled == false</code>." }
{ "name": "highlightConfig", "$ref": "HighlightConfig", "optional": true, "description": "A descriptor for the highlight appearance of hovered-over nodes. May be omitted if <code>enabled == false</code>." },
{ "name": "gridOverlayConfig", "$ref": "GridOverlayConfig", "optional": true, "description": "If provided, used to configure a grid overlay shown during element selection. This overrides DOM.showGridOverlay." },
{ "name": "flexOverlayConfig", "$ref": "FlexOverlayConfig", "optional": true, "description": "If provided, used to configure a flex overlay shown during element selection. This overrides DOM.showFlexOverlay." }
]
},
{
Expand All @@ -435,6 +459,8 @@
"parameters": [
{ "name": "enabled", "type": "boolean", "description": "True to enable inspection mode, false to disable it." },
{ "name": "highlightConfig", "$ref": "HighlightConfig", "optional": true, "description": "A descriptor for the highlight appearance of hovered-over nodes. May be omitted if <code>enabled == false</code>." },
{ "name": "gridOverlayConfig", "$ref": "GridOverlayConfig", "optional": true, "description": "If provided, used to configure a grid overlay shown during element selection. This overrides DOM.showGridOverlay." },
{ "name": "flexOverlayConfig", "$ref": "FlexOverlayConfig", "optional": true, "description": "If provided, used to configure a flex overlay shown during element selection. This overrides DOM.showFlexOverlay." },
{ "name": "showRulers", "type": "boolean", "optional": true, "description": "Whether the rulers should be shown during element selection. This overrides Page.setShowRulers." }
]
},
Expand Down Expand Up @@ -510,12 +536,7 @@
"targetTypes": ["page"],
"parameters": [
{ "name": "nodeId", "$ref": "NodeId", "description": "The node for which a grid overlay should be shown." },
{ "name": "gridColor", "$ref": "RGBAColor", "description": "The primary color to use for the grid overlay." },
{ "name": "showLineNames", "type": "boolean", "optional": true, "description": "Show labels for grid line names. If not specified, the default value is false." },
{ "name": "showLineNumbers", "type": "boolean", "optional": true, "description": "Show labels for grid line numbers. If not specified, the default value is false." },
{ "name": "showExtendedGridLines", "type": "boolean", "optional": true, "description": "Show grid lines that extend beyond the bounds of the grid. If not specified, the default value is false." },
{ "name": "showTrackSizes", "type": "boolean", "optional": true, "description": "Show grid track size information. If not specified, the default value is false." },
{ "name": "showAreaNames", "type": "boolean", "optional": true, "description": "Show labels for grid area names. If not specified, the default value is false." }
{ "name": "gridOverlayConfig", "$ref": "GridOverlayConfig", "description": "Configuration options for the grid overlay." }
]
},
{
Expand All @@ -532,8 +553,7 @@
"targetTypes": ["page"],
"parameters": [
{ "name": "nodeId", "$ref": "NodeId", "description": "The node for which a flex overlay should be shown." },
{ "name": "flexColor", "$ref": "RGBAColor", "description": "The primary color to use for the flex overlay." },
{ "name": "showOrderNumbers", "type": "boolean", "optional": true, "description": "Show labels for flex order. If not specified, the default value is false." }
{ "name": "flexOverlayConfig", "$ref": "FlexOverlayConfig", "description": "Configuration options for the flex overlay." }
]
},
{
Expand Down
2 changes: 2 additions & 0 deletions Source/JavaScriptCore/inspector/scripts/codegen/generator.py
Expand Up @@ -85,6 +85,8 @@ def ucfirst(str):
"Timeline.TimelineEvent": [],
"CSS.CSSProperty": ["priority", "parsedOk", "status"],
"DOM.HighlightConfig": [],
"DOM.GridOverlayConfig": [],
"DOM.FlexOverlayConfig": [],
"DOM.RGBAColor": [],
"DOMStorage.StorageId": [],
"Debugger.BreakpointAction": [],
Expand Down
81 changes: 79 additions & 2 deletions Source/WebCore/inspector/InspectorOverlay.cpp
Expand Up @@ -432,15 +432,31 @@ void InspectorOverlay::paint(GraphicsContext& context)
}

for (const InspectorOverlay::Grid& gridOverlay : m_activeGridOverlays) {
if (m_highlightGridOverlay && gridOverlay.gridNode == m_highlightGridOverlay->gridNode)
continue;

if (auto gridHighlightOverlay = buildGridOverlay(gridOverlay))
drawGridOverlay(context, *gridHighlightOverlay);
}

if (m_highlightGridOverlay) {
if (auto gridHighlightOverlay = buildGridOverlay(*m_highlightGridOverlay))
drawGridOverlay(context, *gridHighlightOverlay);
}

for (const InspectorOverlay::Flex& flexOverlay : m_activeFlexOverlays) {
if (m_highlightFlexOverlay && flexOverlay.flexNode == m_highlightFlexOverlay->flexNode)
continue;

if (auto flexHighlightOverlay = buildFlexOverlay(flexOverlay))
drawFlexOverlay(context, *flexHighlightOverlay);
}

if (m_highlightFlexOverlay) {
if (auto flexHighlightOverlay = buildFlexOverlay(*m_highlightFlexOverlay))
drawFlexOverlay(context, *flexHighlightOverlay);
}

if (!m_paintRects.isEmpty())
drawPaintRects(context, m_paintRects);

Expand All @@ -450,7 +466,7 @@ void InspectorOverlay::paint(GraphicsContext& context)

void InspectorOverlay::getHighlight(InspectorOverlay::Highlight& highlight, InspectorOverlay::CoordinateSystem coordinateSystem)
{
if (!m_highlightNode && !m_highlightQuad && !m_highlightNodeList && !m_activeGridOverlays.size() && !m_activeFlexOverlays.size())
if (!m_highlightNode && !m_highlightQuad && !m_highlightNodeList && !m_highlightGridOverlay && !m_activeGridOverlays.size() && !m_highlightFlexOverlay && !m_activeFlexOverlays.size())
return;

highlight.type = InspectorOverlay::Highlight::Type::None;
Expand All @@ -469,16 +485,34 @@ void InspectorOverlay::getHighlight(InspectorOverlay::Highlight& highlight, Insp
highlight.type = InspectorOverlay::Highlight::Type::Rects;
buildQuadHighlight(*m_highlightQuad, m_quadHighlightConfig, highlight);
}

constexpr bool offsetBoundsByScroll = true;

for (const InspectorOverlay::Grid& gridOverlay : m_activeGridOverlays) {
if (m_highlightGridOverlay && gridOverlay.gridNode == m_highlightGridOverlay->gridNode)
continue;

if (auto gridHighlightOverlay = buildGridOverlay(gridOverlay, offsetBoundsByScroll))
highlight.gridHighlightOverlays.append(*gridHighlightOverlay);
}

if (m_highlightGridOverlay) {
if (auto gridHighlightOverlay = buildGridOverlay(*m_highlightGridOverlay, offsetBoundsByScroll))
highlight.gridHighlightOverlays.append(*gridHighlightOverlay);
}

for (const InspectorOverlay::Flex& flexOverlay : m_activeFlexOverlays) {
if (m_highlightFlexOverlay && flexOverlay.flexNode == m_highlightFlexOverlay->flexNode)
continue;

if (auto flexHighlightOverlay = buildFlexOverlay(flexOverlay))
highlight.flexHighlightOverlays.append(*flexHighlightOverlay);
}

if (m_highlightFlexOverlay) {
if (auto flexHighlightOverlay = buildFlexOverlay(*m_highlightFlexOverlay))
highlight.flexHighlightOverlays.append(*flexHighlightOverlay);
}
}

void InspectorOverlay::hideHighlight()
Expand Down Expand Up @@ -539,7 +573,16 @@ bool InspectorOverlay::shouldShowOverlay() const
{
// Don't show the overlay when m_showRulersDuringElementSelection is true, as it's only supposed
// to have an effect when element selection is active (e.g. a node is hovered).
return m_highlightNode || m_highlightNodeList || m_highlightQuad || m_indicating || m_showPaintRects || m_showRulers || m_activeGridOverlays.size() || m_activeFlexOverlays.size();
return m_highlightNode
|| m_highlightNodeList
|| m_highlightQuad
|| m_highlightGridOverlay
|| m_activeGridOverlays.size()
|| m_highlightFlexOverlay
|| m_activeFlexOverlays.size()
|| m_indicating
|| m_showPaintRects
|| m_showRulers;
}

void InspectorOverlay::update()
Expand Down Expand Up @@ -607,6 +650,23 @@ bool InspectorOverlay::removeGridOverlayForNode(Node& node)
});
}

void InspectorOverlay::showHighlightGridOverlayForNode(Node& node, const InspectorOverlay::Grid::Config& gridOverlayConfig)
{
if (!is<RenderGrid>(node.renderer())) {
hideHighlightGridOverlay();
return;
}

m_highlightGridOverlay = { node, gridOverlayConfig };
update();
}

void InspectorOverlay::hideHighlightGridOverlay()
{
m_highlightGridOverlay = std::nullopt;
update();
}

ErrorStringOr<void> InspectorOverlay::setGridOverlayForNode(Node& node, const InspectorOverlay::Grid::Config& gridOverlayConfig)
{
RenderObject* renderer = node.renderer();
Expand Down Expand Up @@ -647,6 +707,23 @@ bool InspectorOverlay::removeFlexOverlayForNode(Node& node)
});
}

void InspectorOverlay::showHighlightFlexOverlayForNode(Node& node, const InspectorOverlay::Flex::Config& flexOverlayConfig)
{
if (!is<RenderFlexibleBox>(node.renderer())) {
hideHighlightFlexOverlay();
return;
}

m_highlightFlexOverlay = { node, flexOverlayConfig };
update();
}

void InspectorOverlay::hideHighlightFlexOverlay()
{
m_highlightFlexOverlay = std::nullopt;
update();
}

ErrorStringOr<void> InspectorOverlay::setFlexOverlayForNode(Node& node, const InspectorOverlay::Flex::Config& flexOverlayConfig)
{
if (!is<RenderFlexibleBox>(node.renderer()))
Expand Down
8 changes: 8 additions & 0 deletions Source/WebCore/inspector/InspectorOverlay.h
Expand Up @@ -222,10 +222,15 @@ class InspectorOverlay {

// Multiple grid and flex overlays can be active at the same time. These methods
// will fail if the node is not a grid or if the node has been GC'd.

void showHighlightGridOverlayForNode(Node&, const InspectorOverlay::Grid::Config&);
void hideHighlightGridOverlay();
Inspector::ErrorStringOr<void> setGridOverlayForNode(Node&, const InspectorOverlay::Grid::Config&);
Inspector::ErrorStringOr<void> clearGridOverlayForNode(Node&);
void clearAllGridOverlays();

void showHighlightFlexOverlayForNode(Node&, const InspectorOverlay::Flex::Config&);
void hideHighlightFlexOverlay();
Inspector::ErrorStringOr<void> setFlexOverlayForNode(Node&, const InspectorOverlay::Flex::Config&);
Inspector::ErrorStringOr<void> clearFlexOverlayForNode(Node&);
void clearAllFlexOverlays();
Expand Down Expand Up @@ -269,7 +274,10 @@ class InspectorOverlay {
Deque<TimeRectPair> m_paintRects;
Timer m_paintRectUpdateTimer;

std::optional<InspectorOverlay::Grid> m_highlightGridOverlay;
Vector<InspectorOverlay::Grid> m_activeGridOverlays;

std::optional<InspectorOverlay::Flex> m_highlightFlexOverlay;
Vector<InspectorOverlay::Flex> m_activeFlexOverlays;

bool m_indicating { false };
Expand Down

0 comments on commit f51cad2

Please sign in to comment.