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

fix: adding new property to tagged elements that are overlapped by others [TOL-2408] #894

Merged
merged 8 commits into from
Sep 20, 2024
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
import { describe, expect, it } from 'vitest';

import { InspectorModeDataAttributes } from '../types.js';
import { addCalculatedAttributesToTaggedElements, getAllTaggedElements } from '../utils.js';

describe('addCalculatedAttributesToTaggedElements', () => {
const dataEntry = InspectorModeDataAttributes.ENTRY_ID;
const dataField = InspectorModeDataAttributes.FIELD_ID;
const dataLocale = InspectorModeDataAttributes.LOCALE;

const html = (text: string) => {
return new DOMParser().parseFromString(text, 'text/html');
};

describe('visibility attributes', () => {
it('returns isCoveredByOtherElement as false if no element is covering tagged element', () => {
const dom = html(`
<div>
<div
id="entry-1"
${dataEntry}="entry-1"
${dataField}="field-1"
${dataLocale}="locale-1"
></div>
</div>
`);

dom.getElementById('entry-1')!.checkVisibility = () => true;
const entry1Coordinates = {
bottom: 100,
height: 100,
left: 10,
right: 100,
top: 10,
width: 100,
x: 10,
y: 10,
};
const entry1BoundingClientRect = {
...entry1Coordinates,
toJSON: () => entry1Coordinates,
};
dom.getElementById('entry-1')!.getBoundingClientRect = () => entry1BoundingClientRect;

const { taggedElements: elements } = getAllTaggedElements({
root: dom,
options: { locale: 'locale-1' },
});

const entry1 = dom.getElementById('entry-1');

dom.elementFromPoint = (_x: number, _y: number) => {
return entry1;
};

const elementsWithCalculatedAttributes = addCalculatedAttributesToTaggedElements(
elements,
dom,
);
expect(elementsWithCalculatedAttributes).toEqual([
{
attributes: {
entryId: 'entry-1',
environment: undefined,
fieldId: 'field-1',
locale: 'locale-1',
space: undefined,
manuallyTagged: true,
},
element: dom.getElementById('entry-1'),
isVisible: true,
isCoveredByOtherElement: false,
coordinates: entry1BoundingClientRect,
},
]);
});

it('returns isCoveredByOtherElement as true if an element is covering tagged element', () => {
const dom = html(`
<div>
<div
id="entry-1"
${dataEntry}="entry-1"
${dataField}="field-1"
${dataLocale}="locale-1"
></div>
<div
id="covering-element"
style="position: absolute; z-index: 100; top: 0; left: 0; width: 100%; height: 100%;"
></div>
</div>
`);

dom.getElementById('entry-1')!.checkVisibility = () => true;
const entry1Coordinates = {
bottom: 100,
height: 100,
left: 10,
right: 100,
top: 10,
width: 100,
x: 10,
y: 10,
};
const entry1BoundingClientRect = {
...entry1Coordinates,
toJSON: () => entry1Coordinates,
};
dom.getElementById('entry-1')!.getBoundingClientRect = () => entry1BoundingClientRect;

const { taggedElements: elements } = getAllTaggedElements({
root: dom,
options: { locale: 'locale-1' },
});

const coveringElement = dom.getElementById('covering-element');

dom.elementFromPoint = (_x: number, _y: number) => {
return coveringElement;
};

const elementsWithCalculatedAttributes = addCalculatedAttributesToTaggedElements(
elements,
dom,
);
expect(elementsWithCalculatedAttributes).toEqual([
{
attributes: {
entryId: 'entry-1',
environment: undefined,
fieldId: 'field-1',
locale: 'locale-1',
space: undefined,
manuallyTagged: true,
},
element: dom.getElementById('entry-1'),
isVisible: true,
isCoveredByOtherElement: true,
coordinates: entry1BoundingClientRect,
},
]);
});
});
});
31 changes: 10 additions & 21 deletions packages/live-preview-sdk/src/inspectorMode/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@ import { type MessageFromEditor } from '../messages.js';
import {
InspectorModeDataAttributes,
InspectorModeEventMethods,
type InspectorModeAttributes,
type InspectorModeChangedMessage,
} from './types.js';
import { getAllTaggedElements } from './utils.js';
import {
addCalculatedAttributesToTaggedElements,
getAllTaggedElements,
TaggedElement,
} from './utils.js';

export type InspectorModeOptions = {
locale: string;
Expand All @@ -18,13 +21,6 @@ export type InspectorModeOptions = {
ignoreManuallyTaggedElements?: boolean;
};

type TaggedElement = {
attributes: InspectorModeAttributes | null;
coordinates: DOMRect;
element: Element;
isVisible: boolean;
};

export class InspectorMode {
private delay = 300;

Expand Down Expand Up @@ -239,10 +235,11 @@ export class InspectorMode {
{
elements: this.taggedElements.map((taggedElement) => ({
// Important: do not add `element` as it can't be cloned by sendMessage
coordinates: taggedElement.coordinates,
isVisible: taggedElement.isVisible,
coordinates: taggedElement.coordinates!,
isVisible: !!taggedElement.isVisible,
Comment on lines +238 to +239
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we fix the typings when we create the taggedElements?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've added a new type now

PrecaulculatedTaggedElement

This just has the stuff that you can get directly from the tagged element, so element + attributes

The the actual TaggedElement has the calculated state without any conditional properties

attributes: taggedElement.attributes,
isHovered: this.hoveredElement === taggedElement.element,
isCoveredByOtherElement: !!taggedElement.isCoveredByOtherElement,
})),
automaticallyTaggedCount: this.automaticallyTaggedCount,
manuallyTaggedCount: this.manuallyTaggedCount,
Expand All @@ -260,15 +257,7 @@ export class InspectorMode {
options: this.options,
});

const nextElements = taggedElements.map(({ element, attributes }) => ({
element,
coordinates: element.getBoundingClientRect(),
attributes,
isVisible: element.checkVisibility({
checkOpacity: true,
checkVisibilityCSS: true,
}),
}));
const nextElements = addCalculatedAttributesToTaggedElements(taggedElements);

if (isEqual(nextElements, this.taggedElements)) {
return;
Expand All @@ -279,7 +268,7 @@ export class InspectorMode {
this.observersCB = [];

// update elements and watch them
this.taggedElements = nextElements;
this.taggedElements = nextElements as TaggedElement[];
taggedElements.forEach(({ element }) => this.observe(element));

// update the counters for telemetry
Expand Down
56 changes: 55 additions & 1 deletion packages/live-preview-sdk/src/inspectorMode/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,13 @@ export type AutoTaggedElement<T = Node> = {
sourceMap: SourceMapMetadata;
};

interface TaggedElement {
export interface TaggedElement {
element: Element;
attributes: InspectorModeAttributes | null;
isHovered?: boolean;
isVisible?: boolean;
coordinates?: DOMRect;
isCoveredByOtherElement?: boolean;
}

const isTaggedElement = (node?: Node | null): boolean => {
Expand Down Expand Up @@ -271,3 +275,53 @@ export function getAllTaggedEntries({
),
];
}

const isElementOverlapped = (element: Element, coordinates: DOMRect, root = window.document) => {
const { top, right, bottom, left } = coordinates;
const topLeft = root.elementFromPoint(left, top);
const topRight = root.elementFromPoint(right, top);
const bottomLeft = root.elementFromPoint(left, bottom);
const bottomRight = root.elementFromPoint(right, bottom);

return (
topLeft === element || topRight === element || bottomLeft === element || bottomRight === element
);
};

const addVisibilityToTaggedElements = (
taggedElements: Partial<TaggedElement>[],
root = window.document,
) =>
taggedElements.map((taggedElement) => ({
...taggedElement,
isVisible: taggedElement.element!.checkVisibility({
checkOpacity: true,
checkVisibilityCSS: true,
}),
isCoveredByOtherElement: !isElementOverlapped(
taggedElement.element!,
taggedElement.coordinates!,
root,
),
}));

const addCoordinatesToTaggedElements = (
taggedElements: Partial<TaggedElement>[],
): Partial<TaggedElement>[] =>
taggedElements.map(({ element, attributes }) => ({
element,
coordinates: element!.getBoundingClientRect(),
attributes,
}));

/**
* Applies the attributes that we cannot simply get from the tagged elements itself
* but need to calculate based on the current state of the document
*/
export const addCalculatedAttributesToTaggedElements = (
taggedElements: Partial<TaggedElement>[],
root = window.document,
): TaggedElement[] => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
): TaggedElement[] => {
): PrecaulculatedTaggedElement[] => {

Copy link
Contributor

Choose a reason for hiding this comment

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

same then in 332

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No this should use the regular one as it now has added the calculations

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've merged for now, let me know I can follow up later on this if you disagree

const taggedElementWithCoordinates = addCoordinatesToTaggedElements(taggedElements);
return addVisibilityToTaggedElements(taggedElementWithCoordinates, root) as TaggedElement[];
};
Loading