Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: simplify SVG path filtering and parent selection #899

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
3 changes: 2 additions & 1 deletion apps/studio/common/helpers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ export function isValidHtmlElement(element: Element): boolean {
element.nodeType === Node.ELEMENT_NODE &&
!DOM_IGNORE_TAGS.includes(element.tagName) &&
!element.hasAttribute(EditorAttributes.DATA_ONLOOK_IGNORE) &&
(element as HTMLElement).style.display !== 'none'
(element as HTMLElement).style.display !== 'none' &&
!(element.closest('svg') && element !== element.closest('svg'))
);
}

Expand Down
11 changes: 7 additions & 4 deletions apps/studio/electron/preload/webview/dom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,13 @@ export function buildLayerTree(root: HTMLElement): Map<string, LayerNode> | null

const layerMap = new Map<string, LayerNode>();
const treeWalker = document.createTreeWalker(root, NodeFilter.SHOW_ELEMENT, {
acceptNode: (node: Node) =>
isValidHtmlElement(node as HTMLElement)
Copy link
Contributor

@Kitenite Kitenite Dec 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could just update this to check for the parent being svg and node being path

export function isValidHtmlElement(element: Element): boolean {
return (
element &&
element instanceof Node &&
element.nodeType === Node.ELEMENT_NODE &&
!DOM_IGNORE_TAGS.includes(element.tagName) &&
!element.hasAttribute(EditorAttributes.DATA_ONLOOK_IGNORE) &&
(element as HTMLElement).style.display !== 'none'
);
}

? NodeFilter.FILTER_ACCEPT
: NodeFilter.FILTER_SKIP,
acceptNode: (node: Node) => {
const element = node as HTMLElement;
if (!isValidHtmlElement(element)) {
return NodeFilter.FILTER_SKIP;
}
return NodeFilter.FILTER_ACCEPT;
},
});

// Process root node
Expand Down
1 change: 1 addition & 0 deletions apps/studio/electron/preload/webview/elements/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export const getDeepElement = (x: number, y: number): Element | undefined => {
if (!el) {
return;
}

const crawlShadows = (node: Element): Element => {
if (node?.shadowRoot) {
const potential = node.shadowRoot.elementFromPoint(x, y);
Expand Down
11 changes: 9 additions & 2 deletions apps/studio/electron/preload/webview/elements/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { EditorAttributes } from '@onlook/models/constants';
import type { DomElement } from '@onlook/models/element';
import { getDomElement } from './helpers';
import { elementFromDomId } from '/common/helpers';
import { elementFromDomId, isValidHtmlElement } from '/common/helpers';

export const getDomElementByDomId = (domId: string, style: boolean): DomElement => {
const el = elementFromDomId(domId) || document.body;
Expand All @@ -15,9 +15,16 @@ export const getElementAtLoc = (x: number, y: number, getStyle: boolean): DomEle

const getDeepElement = (x: number, y: number): Element | undefined => {
const el = document.elementFromPoint(x, y);
if (!el) {
if (!el || !isValidHtmlElement(el)) {
return;
}

// If the element is an SVG child, return its parent SVG element
const parentSvg = el.closest('svg');
if (parentSvg && el !== parentSvg) {
return parentSvg;
}

const crawlShadows = (node: Element): Element => {
if (node?.shadowRoot) {
const potential = node.shadowRoot.elementFromPoint(x, y);
Expand Down
62 changes: 47 additions & 15 deletions apps/studio/src/lib/editor/engine/element/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,25 +25,39 @@ export class ElementManager {
this.selectedElements = elements;
}

mouseover(domEl: DomElement, webview: Electron.WebviewTag) {
async mouseover(domEl: DomElement, webview: Electron.WebviewTag) {
if (!domEl) {
this.editorEngine.overlay.removeHoverRect();
this.clearHoveredElement();
return;
}
if (this.hoveredElement && this.hoveredElement.domId === domEl.domId) {

const elementToHover = await webview.executeJavaScript(`
(function() {
const el = document.querySelector('[data-onlook-dom-id="${domEl.domId}"]');
const parentSvg = el?.closest('svg');
return parentSvg && el !== parentSvg
? window.api?.getDomElementByDomId(parentSvg.getAttribute('data-onlook-dom-id'))
: window.api?.getDomElementByDomId('${domEl.domId}');
})()
`);

if (
!elementToHover ||
(this.hoveredElement && this.hoveredElement.domId === elementToHover.domId)
) {
return;
}

const webviewEl: DomElement = {
...domEl,
...elementToHover,
webviewId: webview.id,
};
const adjustedRect = this.editorEngine.overlay.adaptRectFromSourceElement(
webviewEl.rect,
webview,
);
const isComponent = !!domEl.instanceId;
const isComponent = !!elementToHover.instanceId;
this.editorEngine.overlay.updateHoverRect(adjustedRect, isComponent);
this.setHoveredElement(webviewEl);
}
Expand All @@ -57,8 +71,8 @@ export class ElementManager {
const selectedEl = this.selected[0];
const hoverEl = this.hovered;

const webViewId = selectedEl.webviewId;
const webview = this.editorEngine.webviews.getWebview(webViewId);
const webviewId = selectedEl.webviewId;
const webview = this.editorEngine.webviews.getWebview(webviewId);
if (!webview) {
return;
}
Expand All @@ -75,7 +89,7 @@ export class ElementManager {
this.editorEngine.overlay.updateMeasurement(selectedRect, hoverRect);
}

shiftClick(domEl: DomElement, webview: Electron.WebviewTag) {
async shiftClick(domEl: DomElement, webview: Electron.WebviewTag) {
const selectedEls = this.selected;
const isAlreadySelected = selectedEls.some((el) => el.domId === domEl.domId);
let newSelectedEls: DomElement[] = [];
Expand All @@ -84,26 +98,44 @@ export class ElementManager {
} else {
newSelectedEls = [...selectedEls, domEl];
}
this.click(newSelectedEls, webview);
await this.click(newSelectedEls, webview);
}

click(domEls: DomElement[], webview: Electron.WebviewTag) {
async click(domEls: DomElement[], webview: Electron.WebviewTag) {
this.editorEngine.overlay.removeClickedRects();
this.clearSelectedElements();

for (const domEl of domEls) {
const elementToSelect = await webview.executeJavaScript(`
(function() {
const el = document.querySelector('[data-onlook-dom-id="${domEl.domId}"]');
const parentSvg = el?.closest('svg');
return parentSvg && el !== parentSvg
? window.api?.getDomElementByDomId(parentSvg.getAttribute('data-onlook-dom-id'))
: window.api?.getDomElementByDomId('${domEl.domId}');
})()
`);

if (!elementToSelect) {
continue;
}

const adjustedRect = this.editorEngine.overlay.adaptRectFromSourceElement(
domEl.rect,
elementToSelect.rect,
webview,
);
const isComponent = !!domEl.instanceId;
this.editorEngine.overlay.addClickRect(adjustedRect, domEl.styles, isComponent);
this.addSelectedElement(domEl);
const isComponent = !!elementToSelect.instanceId;
this.editorEngine.overlay.addClickRect(
adjustedRect,
elementToSelect.styles,
isComponent,
);
this.addSelectedElement(elementToSelect);
}
}

refreshSelectedElements(webview: Electron.WebviewTag) {
this.debouncedRefreshClickedElements(webview);
async refreshSelectedElements(webview: Electron.WebviewTag) {
await this.debouncedRefreshClickedElements(webview);
}

setHoveredElement(element: DomElement) {
Expand Down
37 changes: 36 additions & 1 deletion apps/studio/src/lib/editor/engine/overlay/rect.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { isValidHtmlElement } from '/common/helpers';
import { colors } from '@onlook/ui/tokens';

import { EditorAttributes } from '@onlook/models/constants';
import { nanoid } from 'nanoid/non-secure';

Expand Down Expand Up @@ -64,6 +64,23 @@ export class HoverRect extends RectImpl {
}

render(rectDimensions: RectDimensions, isComponent?: boolean) {
const { width, height, top, left } = rectDimensions;
const targetEl = document.elementFromPoint(left + width / 2, top + height / 2);
if (!targetEl || !isValidHtmlElement(targetEl)) {
return;
}

const parentSvg = targetEl.closest('svg');
if (parentSvg && targetEl !== parentSvg) {
const rect = parentSvg.getBoundingClientRect();
rectDimensions = {
width: rect.width,
height: rect.height,
top: rect.top,
left: rect.left,
};
}

super.render(rectDimensions, isComponent);
}
}
Expand Down Expand Up @@ -352,6 +369,24 @@ export class ClickRect extends RectImpl {
},
isComponent?: boolean,
) {
// Don't render if element is not valid
const targetEl = document.elementFromPoint(left + width / 2, top + height / 2);
if (!targetEl || !isValidHtmlElement(targetEl)) {
return;
}

// If clicking SVG child, get parent SVG dimensions
const parentSvg = targetEl.closest('svg');
if (parentSvg && targetEl !== parentSvg) {
const rect = parentSvg.getBoundingClientRect();
width = rect.width;
height = rect.height;
top = rect.top;
left = rect.left;
// For SVG elements, we don't show margins
margin = '0px';
}

// Sometimes a selected element can be removed. We handle this gracefully.
try {
this.updateMargin(margin, { width, height });
Expand Down