From: Dan Brown Date: Sun, 15 Jun 2025 12:55:42 +0000 (+0100) Subject: Lexical: Fixed media resize handling X-Git-Tag: v25.05.1~1^2~4^2~2 X-Git-Url: http://source.bookstackapp.com/bookstack/commitdiff_plain/8d4b8ff4f375c014d36ccbf034dfc77bb8481804 Lexical: Fixed media resize handling - Updating height/width setting to clear any inline CSS width/height rules which would override and prevent resizes showing. This was common when switching media from old editor. Added test to cover. - Updated resizer to track node so that it is retained & displayed across node DOM changes, which was previously causing the resizer/focus to disappear. --- diff --git a/resources/js/wysiwyg/lexical/core/__tests__/utils/index.ts b/resources/js/wysiwyg/lexical/core/__tests__/utils/index.ts index 6a8e45724..e18ef9756 100644 --- a/resources/js/wysiwyg/lexical/core/__tests__/utils/index.ts +++ b/resources/js/wysiwyg/lexical/core/__tests__/utils/index.ts @@ -38,6 +38,7 @@ import {DetailsNode} from "@lexical/rich-text/LexicalDetailsNode"; import {EditorUiContext} from "../../../../ui/framework/core"; import {EditorUIManager} from "../../../../ui/framework/manager"; import {ImageNode} from "@lexical/rich-text/LexicalImageNode"; +import {MediaNode} from "@lexical/rich-text/LexicalMediaNode"; type TestEnv = { readonly container: HTMLDivElement; @@ -487,6 +488,7 @@ export function createTestContext(): EditorUiContext { theme: {}, nodes: [ ImageNode, + MediaNode, ] }); diff --git a/resources/js/wysiwyg/lexical/rich-text/LexicalMediaNode.ts b/resources/js/wysiwyg/lexical/rich-text/LexicalMediaNode.ts index 1dd159f51..6e9c24717 100644 --- a/resources/js/wysiwyg/lexical/rich-text/LexicalMediaNode.ts +++ b/resources/js/wysiwyg/lexical/rich-text/LexicalMediaNode.ts @@ -8,7 +8,7 @@ import { } from 'lexical'; import type {EditorConfig} from "lexical/LexicalEditor"; -import {el, setOrRemoveAttribute, sizeToPixels} from "../../utils/dom"; +import {el, setOrRemoveAttribute, sizeToPixels, styleMapToStyleString, styleStringToStyleMap} from "../../utils/dom"; import { CommonBlockAlignment, deserializeCommonBlockNode, setCommonBlockPropsFromElement, @@ -46,6 +46,19 @@ function filterAttributes(attributes: Record): Record, styleName: string): Record { + const attrCopy = Object.assign({}, attributes); + if (!attributes.style) { + return attrCopy; + } + + const map = styleStringToStyleMap(attributes.style); + map.delete(styleName); + + attrCopy.style = styleMapToStyleString(map); + return attrCopy; +} + function domElementToNode(tag: MediaNodeTag, element: HTMLElement): MediaNode { const node = $createMediaNode(tag); @@ -118,7 +131,7 @@ export class MediaNode extends ElementNode { getAttributes(): Record { const self = this.getLatest(); - return self.__attributes; + return Object.assign({}, self.__attributes); } setSources(sources: MediaNodeSource[]) { @@ -132,7 +145,7 @@ export class MediaNode extends ElementNode { } setSrc(src: string): void { - const attrs = Object.assign({}, this.getAttributes()); + const attrs = this.getAttributes(); if (this.__tag ==='object') { attrs.data = src; } else { @@ -142,11 +155,13 @@ export class MediaNode extends ElementNode { } setWidthAndHeight(width: string, height: string): void { - const attrs = Object.assign( - {}, + let attrs: Record = Object.assign( this.getAttributes(), {width, height}, ); + + attrs = removeStyleFromAttributes(attrs, 'width'); + attrs = removeStyleFromAttributes(attrs, 'height'); this.setAttributes(attrs); } @@ -185,8 +200,8 @@ export class MediaNode extends ElementNode { return; } - const attrs = Object.assign({}, this.getAttributes(), {height}); - this.setAttributes(attrs); + const attrs = Object.assign(this.getAttributes(), {height}); + this.setAttributes(removeStyleFromAttributes(attrs, 'height')); } getHeight(): number { @@ -195,8 +210,9 @@ export class MediaNode extends ElementNode { } setWidth(width: number): void { - const attrs = Object.assign({}, this.getAttributes(), {width}); - this.setAttributes(attrs); + const existingAttrs = this.getAttributes(); + const attrs: Record = Object.assign(existingAttrs, {width}); + this.setAttributes(removeStyleFromAttributes(attrs, 'width')); } getWidth(): number { diff --git a/resources/js/wysiwyg/lexical/rich-text/__tests__/unit/LexicalMediaNode.test.ts b/resources/js/wysiwyg/lexical/rich-text/__tests__/unit/LexicalMediaNode.test.ts new file mode 100644 index 000000000..c55ae669e --- /dev/null +++ b/resources/js/wysiwyg/lexical/rich-text/__tests__/unit/LexicalMediaNode.test.ts @@ -0,0 +1,31 @@ +import {createTestContext} from "lexical/__tests__/utils"; +import {$createMediaNode} from "@lexical/rich-text/LexicalMediaNode"; + + +describe('LexicalMediaNode', () => { + + test('setWidth/setHeight/setWidthAndHeight functions remove relevant styles', () => { + const {editor} = createTestContext(); + editor.updateAndCommit(() => { + const mediaMode = $createMediaNode('video'); + const defaultStyles = {style: 'width:20px;height:40px;color:red'}; + + mediaMode.setAttributes(defaultStyles); + mediaMode.setWidth(60); + expect(mediaMode.getWidth()).toBe(60); + expect(mediaMode.getAttributes().style).toBe('height:40px;color:red'); + + mediaMode.setAttributes(defaultStyles); + mediaMode.setHeight(77); + expect(mediaMode.getHeight()).toBe(77); + expect(mediaMode.getAttributes().style).toBe('width:20px;color:red'); + + mediaMode.setAttributes(defaultStyles); + mediaMode.setWidthAndHeight('6', '7'); + expect(mediaMode.getWidth()).toBe(6); + expect(mediaMode.getHeight()).toBe(7); + expect(mediaMode.getAttributes().style).toBe('color:red'); + }); + }); + +}); \ No newline at end of file diff --git a/resources/js/wysiwyg/ui/framework/helpers/node-resizer.ts b/resources/js/wysiwyg/ui/framework/helpers/node-resizer.ts index 934ac5763..8b0a1d2d7 100644 --- a/resources/js/wysiwyg/ui/framework/helpers/node-resizer.ts +++ b/resources/js/wysiwyg/ui/framework/helpers/node-resizer.ts @@ -13,7 +13,7 @@ function isNodeWithSize(node: LexicalNode): node is NodeHasSize&LexicalNode { class NodeResizer { protected context: EditorUiContext; protected resizerDOM: HTMLElement|null = null; - protected targetDOM: HTMLElement|null = null; + protected targetNode: LexicalNode|null = null; protected scrollContainer: HTMLElement; protected mouseTracker: MouseDragTracker|null = null; @@ -38,12 +38,7 @@ class NodeResizer { if (nodes.length === 1 && isNodeWithSize(nodes[0])) { const node = nodes[0]; - const nodeKey = node.getKey(); - let nodeDOM = this.context.editor.getElementByKey(nodeKey); - - if (nodeDOM && nodeDOM.nodeName === 'SPAN') { - nodeDOM = nodeDOM.firstElementChild as HTMLElement; - } + let nodeDOM = this.getTargetDOM(node) if (nodeDOM) { this.showForNode(node, nodeDOM); @@ -51,7 +46,19 @@ class NodeResizer { } } - onTargetDOMLoad(): void { + protected getTargetDOM(targetNode: LexicalNode|null): HTMLElement|null { + if (targetNode == null) { + return null; + } + + let nodeDOM = this.context.editor.getElementByKey(targetNode.__key) + if (nodeDOM && nodeDOM.nodeName === 'SPAN') { + nodeDOM = nodeDOM.firstElementChild as HTMLElement; + } + return nodeDOM; + } + + protected onTargetDOMLoad(): void { this.updateResizerPosition(); } @@ -62,7 +69,7 @@ class NodeResizer { protected showForNode(node: NodeHasSize&LexicalNode, targetDOM: HTMLElement) { this.resizerDOM = this.buildDOM(); - this.targetDOM = targetDOM; + this.targetNode = node; let ghost = el('span', {class: 'editor-node-resizer-ghost'}); if ($isImageNode(node)) { @@ -83,12 +90,13 @@ class NodeResizer { } protected updateResizerPosition() { - if (!this.resizerDOM || !this.targetDOM) { + const targetDOM = this.getTargetDOM(this.targetNode); + if (!this.resizerDOM || !targetDOM) { return; } const scrollAreaRect = this.scrollContainer.getBoundingClientRect(); - const nodeRect = this.targetDOM.getBoundingClientRect(); + const nodeRect = targetDOM.getBoundingClientRect(); const top = nodeRect.top - (scrollAreaRect.top - this.scrollContainer.scrollTop); const left = nodeRect.left - scrollAreaRect.left; @@ -110,7 +118,7 @@ class NodeResizer { protected hide() { this.mouseTracker?.teardown(); this.resizerDOM?.remove(); - this.targetDOM = null; + this.targetNode = null; this.activeSelection = ''; this.loadAbortController.abort(); } @@ -126,7 +134,7 @@ class NodeResizer { }, handleElems); } - setupTracker(container: HTMLElement, node: NodeHasSize, nodeDOM: HTMLElement): MouseDragTracker { + setupTracker(container: HTMLElement, node: NodeHasSize&LexicalNode, nodeDOM: HTMLElement): MouseDragTracker { let startingWidth: number = 0; let startingHeight: number = 0; let startingRatio: number = 0; @@ -179,10 +187,13 @@ class NodeResizer { _this.context.editor.update(() => { node.setWidth(size.width); node.setHeight(hasHeight ? size.height : 0); - _this.context.manager.triggerLayoutUpdate(); - requestAnimationFrame(() => { - _this.updateResizerPosition(); - }) + }, { + onUpdate: () => { + requestAnimationFrame(() => { + _this.context.manager.triggerLayoutUpdate(); + _this.updateResizerPosition(); + }); + } }); _this.resizerDOM?.classList.remove('active'); } diff --git a/resources/js/wysiwyg/utils/dom.ts b/resources/js/wysiwyg/utils/dom.ts index bbb07cb41..8bacd1002 100644 --- a/resources/js/wysiwyg/utils/dom.ts +++ b/resources/js/wysiwyg/utils/dom.ts @@ -52,12 +52,19 @@ export type StyleMap = Map; /** * Creates a map from an element's styles. * Uses direct attribute value string handling since attempting to iterate - * over .style will expand out any shorthand properties (like 'padding') making + * over .style will expand out any shorthand properties (like 'padding') * rather than being representative of the actual properties set. */ export function extractStyleMapFromElement(element: HTMLElement): StyleMap { - const map: StyleMap = new Map(); const styleText= element.getAttribute('style') || ''; + return styleStringToStyleMap(styleText); +} + +/** + * Convert string-formatted styles into a StyleMap. + */ +export function styleStringToStyleMap(styleText: string): StyleMap { + const map: StyleMap = new Map(); const rules = styleText.split(';'); for (const rule of rules) { @@ -72,6 +79,17 @@ export function extractStyleMapFromElement(element: HTMLElement): StyleMap { return map; } +/** + * Convert a StyleMap into inline string style text. + */ +export function styleMapToStyleString(map: StyleMap): string { + const parts = []; + for (const [style, value] of map.entries()) { + parts.push(`${style}:${value}`); + } + return parts.join(';'); +} + export function setOrRemoveAttribute(element: HTMLElement, name: string, value: string|null|undefined) { if (value) { element.setAttribute(name, value); diff --git a/resources/sass/_editor.scss b/resources/sass/_editor.scss index 4112f6288..633fa78a6 100644 --- a/resources/sass/_editor.scss +++ b/resources/sass/_editor.scss @@ -454,7 +454,7 @@ body.editor-is-fullscreen { .editor-media-wrap { display: inline-block; cursor: not-allowed; - iframe { + iframe, video { pointer-events: none; } &.align-left {