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

Fixed deleting element around array/map expressions #786

Merged
merged 18 commits into from
Dec 7, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 90 additions & 10 deletions apps/studio/electron/main/code/diff/remove.ts
Original file line number Diff line number Diff line change
@@ -1,28 +1,108 @@
import type { NodePath } from '@babel/traverse';
import type * as t from '@babel/types';
import { jsxFilter } from './helpers';
import * as t from '@babel/types';
import { addKeyToElement } from './helpers';
import { assertNever } from '/common/helpers';
import { InsertPos } from '@onlook/models/editor';
import type { CodeAction } from '@onlook/models/actions';

export function removeElementFromNode(path: NodePath<t.JSXElement>, element: CodeAction): void {
const children = path.node.children;
const jsxElements = children.filter(jsxFilter);

children.forEach((child, index) => {
if (t.isJSXElement(child)) {
addKeyToElement(child);
}
});

switch (element.location.position) {
case InsertPos.INDEX:
removeElementAtIndex(element.location.index, jsxElements, children);
case InsertPos.INDEX: {
let currentStaticCount = -1;
let targetChildIndex = -1;

// Find occurrence count in children
for (let i = 0; i < children.length; i++) {
const child = children[i];
if (t.isJSXElement(child)) {
const keyAttribute = child.openingElement.attributes.find(
(attr) => t.isJSXAttribute(attr) && attr.name.name === 'key',
);

if (keyAttribute && t.isJSXAttribute(keyAttribute)) {
const keyValue = (keyAttribute.value as any)?.value;
if (
keyValue &&
typeof keyValue === 'string' &&
keyValue.startsWith('onlook-')
) {
currentStaticCount++;

if (currentStaticCount === element.location.staticIndex) {
targetChildIndex = i;
break;
}
}
}
}
}

if (targetChildIndex !== -1) {
children.splice(targetChildIndex, 1);
} else {
console.error(
'Could not find static element at index:',
element.location.staticIndex,
);
}
break;
case InsertPos.APPEND:
removeElementAtIndex(jsxElements.length - 1, jsxElements, children);
}
case InsertPos.APPEND: {
// Find last element with onlook- key
for (let i = children.length - 1; i >= 0; i--) {
const child = children[i];
if (t.isJSXElement(child)) {
const keyAttr = child.openingElement.attributes.find(
(attr) => t.isJSXAttribute(attr) && attr.name.name === 'key',
);
const keyValue = (keyAttr as any)?.value?.value;
if (
keyValue &&
typeof keyValue === 'string' &&
keyValue.startsWith('onlook-')
) {
children.splice(i, 1);
break;
}
}
}
break;
case InsertPos.PREPEND:
removeElementAtIndex(0, jsxElements, children);
}
case InsertPos.PREPEND: {
// Find first element with onlook- key
for (let i = 0; i < children.length; i++) {
const child = children[i];
if (t.isJSXElement(child)) {
const keyAttr = child.openingElement.attributes.find(
(attr) => t.isJSXAttribute(attr) && attr.name.name === 'key',
);
const keyValue = (keyAttr as any)?.value?.value;
if (
keyValue &&
typeof keyValue === 'string' &&
keyValue.startsWith('onlook-')
) {
children.splice(i, 1);
break;
}
}
}
break;
default:
}
default: {
console.error(`Unhandled position: ${element.location.position}`);
assertNever(element.location.position);
}
}

path.stop();
}

Expand Down
7 changes: 6 additions & 1 deletion apps/studio/electron/preload/webview/api.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import { contextBridge } from 'electron';
import { processDom } from './dom';
import { getElementAtLoc, getElementWithSelector } from './elements';
import { getActionElementBySelector, getActionElementLocation } from './elements/dom/helpers';
import {
getActionElementBySelector,
getActionElementLocation,
isDynamicElement,
} from './elements/dom/helpers';
import { getInsertLocation } from './elements/dom/insert';
import { getRemoveActionFromSelector } from './elements/dom/remove';
import { isElementInserted } from './elements/helpers';
Expand All @@ -20,6 +24,7 @@ export function setApi() {
getComputedStyleBySelector: getComputedStyleBySelector,
getActionElementLocation: getActionElementLocation,
getActionElementBySelector: getActionElementBySelector,
isDynamicElement: isDynamicElement,

// Theme
getTheme: getTheme,
Expand Down
59 changes: 59 additions & 0 deletions apps/studio/electron/preload/webview/dom.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,62 @@
import { ipcRenderer } from 'electron';
import { EditorAttributes } from '@onlook/models/constants';
import { getOrAssignUuid } from './elements/helpers';
import { WebviewChannels } from '@onlook/models/constants';
import { getUniqueSelector, isValidHtmlElement } from '/common/helpers';
import type { LayerNode } from '@onlook/models/element';

function markDynamicElements(root: HTMLElement) {
const containers = root.querySelectorAll(`[${EditorAttributes.DATA_ONLOOK_ID}]`);

containers.forEach((container) => {
const children = Array.from(container.children);
if (children.length < 2) {
return;
}

const groups = children.reduce(
(acc, child) => {
if (!isValidHtmlElement(child)) {
return acc;
}

const signature = [
child.tagName,
child.className,
child.getAttribute(EditorAttributes.DATA_ONLOOK_ID),
Copy link
Contributor

Choose a reason for hiding this comment

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

My understanding: We're making an assumption that if an element has the same data-onlook-id as its sibling, it would be part of an array?

One edge case is if an element is an instance and it's used twice, these would have the same signature?

export default function Page() {
    return (
        <div>
            <Instance />
            <Instance />
        </div>
    );
}

function Instance() {
    return <div>Instance</div>;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Thought this is really close! I wonder if there's something we can do on the plugin side to populate these attributes

Array.from(child.attributes)
.filter((attr) => attr.name.startsWith('data-') || attr.name === 'key')
.map((attr) => attr.name)
.sort()
.join(','),
].join('|');

if (!acc[signature]) {
acc[signature] = [];
}
acc[signature].push(child);
return acc;
},
{} as Record<string, Element[]>,
);

// Mark elements that is part of a map/iteration, dynamic
Object.values(groups).forEach((group) => {
if (group.length > 1) {
group.forEach((element) => {
if (element instanceof HTMLElement) {
element.setAttribute(EditorAttributes.DATA_ONLOOK_DYNAMIC, 'true');
element.setAttribute(EditorAttributes.DATA_ONLOOK_DYNAMIC_TYPE, 'map');
}
});
}
});
});
}

export function processDom(root: HTMLElement = document.body) {
markDynamicElements(root);

const layerTree = buildLayerTree(root);
if (!layerTree) {
console.error('Error building layer tree, root element is null');
Expand Down Expand Up @@ -65,6 +117,11 @@ export function buildLayerTree(root: HTMLElement): LayerNode | null {
function processNode(node: HTMLElement): LayerNode {
getOrAssignUuid(node);

const isDynamic = node.hasAttribute(EditorAttributes.DATA_ONLOOK_DYNAMIC);
const dynamicType: any = isDynamic
? node.getAttribute(EditorAttributes.DATA_ONLOOK_DYNAMIC_TYPE)
: null;

const textContent = Array.from(node.childNodes)
.map((node) => (node.nodeType === Node.TEXT_NODE ? node.textContent : ''))
.join(' ')
Expand All @@ -78,5 +135,7 @@ function processNode(node: HTMLElement): LayerNode {
textContent: textContent || '',
tagName: node.tagName.toLowerCase(),
isVisible: style.visibility !== 'hidden',
isDynamic,
dynamicType,
};
}
18 changes: 18 additions & 0 deletions apps/studio/electron/preload/webview/elements/dom/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,21 @@ export function getActionElementLocation(selector: string): ActionElementLocatio
index,
};
}

export function isDynamicElement(selector: string): { isDynamic: boolean; type: string | null } {
const el = document.querySelector(selector) as HTMLElement | null;

if (!el) {
return { isDynamic: false, type: null };
}

const isDynamic =
el.hasAttribute('data-onlook-dynamic') || el.closest('[data-onlook-dynamic]') !== null;

const type = el.getAttribute('data-onlook-dynamic-type');

return {
isDynamic,
type: isDynamic ? type : null,
};
}
26 changes: 25 additions & 1 deletion apps/studio/electron/preload/webview/elements/dom/remove.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,17 @@ export function getRemoveActionFromSelector(
return;
}

const location = getElementLocation(el);
const parent = el.parentElement;
if (!parent) {
console.error('Parent element not found for:', selector);
return;
}

const location = {
...getElementLocation(el)!,
staticIndex: getElementStaticIndex(parent, el),
};

if (!location) {
console.error('Failed to get location for element:', el);
return;
Expand All @@ -39,3 +49,17 @@ export function getRemoveActionFromSelector(
element: actionEl,
};
}

function getElementStaticIndex(parent: Element, element: Element): number {
const children = Array.from(parent.children);

const index = children.indexOf(element);

//determines position occurrence
const staticIndex =
children
.slice(0, index + 1)
.filter((el) => !el.hasAttribute(EditorAttributes.DATA_ONLOOK_DYNAMIC)).length - 1;

return staticIndex;
}
11 changes: 11 additions & 0 deletions apps/studio/src/lib/editor/engine/element/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,17 @@ export class ElementManager {
return;
}

const { isDynamic, type } = await webview.executeJavaScript(
`window.api?.isDynamicElement('${escapeSelector(selectedEl.selector)}')`,
);

if (isDynamic) {
alert(
`This element is a generated element and cannot be deleted because its part of a ${type} expression`,
);
return;
}

const removeAction = (await webview.executeJavaScript(
`window.api?.getRemoveActionFromSelector('${escapeSelector(selectedEl.selector)}', '${webviewId}')`,
)) as RemoveElementAction | undefined;
Expand Down
1 change: 1 addition & 0 deletions packages/models/src/actions/editor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export const ActionElementLocationSchema = z.object({
position: z.nativeEnum(InsertPos),
targetSelector: z.string(),
index: z.number(),
staticIndex: z.number().optional(),
});

export const MoveActionLocationSchema = ActionElementLocationSchema.extend({
Expand Down
2 changes: 2 additions & 0 deletions packages/models/src/constants/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ export enum EditorAttributes {
DATA_ONLOOK_NEW_INDEX = 'data-onlook-new-index',
DATA_ONLOOK_EDITING_TEXT = 'data-onlook-editing-text',
DATA_ONLOOK_ORIGINAL_CONTENT = 'data-onlook-original-content',
DATA_ONLOOK_DYNAMIC = 'data-onlook-dynamic',
DATA_ONLOOK_DYNAMIC_TYPE = 'data-onlook-dynamic-type',
}

export enum WebviewChannels {
Expand Down
2 changes: 2 additions & 0 deletions packages/models/src/element/layers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ const baseLayerNodeSchema = z.object({
textContent: z.string(),
tagName: z.string(),
isVisible: z.boolean(),
isDynamic: z.boolean().optional(),
dynamicType: z.enum(['map', 'conditional', 'iteration', 'unknown']).optional(),
});

export const LayerNodeSchema: z.ZodType<LayerNode> = baseLayerNodeSchema.extend({
Expand Down