Skip to content

Commit 8d4b8ff

Browse files
ssddanbrownssddanbrown
committed
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.
1 parent 77a8861 commit 8d4b8ff

File tree

6 files changed

+107
-29
lines changed

6 files changed

+107
-29
lines changed

‎resources/js/wysiwyg/lexical/core/__tests__/utils/index.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import {DetailsNode} from "@lexical/rich-text/LexicalDetailsNode";
3838
import {EditorUiContext} from "../../../../ui/framework/core";
3939
import {EditorUIManager} from "../../../../ui/framework/manager";
4040
import {ImageNode} from "@lexical/rich-text/LexicalImageNode";
41+
import {MediaNode} from "@lexical/rich-text/LexicalMediaNode";
4142

4243
type TestEnv = {
4344
readonly container: HTMLDivElement;
@@ -487,6 +488,7 @@ export function createTestContext(): EditorUiContext {
487488
theme: {},
488489
nodes: [
489490
ImageNode,
491+
MediaNode,
490492
]
491493
});
492494

‎resources/js/wysiwyg/lexical/rich-text/LexicalMediaNode.ts‎

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {
88
} from 'lexical';
99
import type {EditorConfig} from "lexical/LexicalEditor";
1010

11-
import {el, setOrRemoveAttribute, sizeToPixels} from "../../utils/dom";
11+
import {el, setOrRemoveAttribute, sizeToPixels, styleMapToStyleString, styleStringToStyleMap} from "../../utils/dom";
1212
import {
1313
CommonBlockAlignment, deserializeCommonBlockNode,
1414
setCommonBlockPropsFromElement,
@@ -46,6 +46,19 @@ function filterAttributes(attributes: Record<string, string>): Record<string, st
4646
return filtered;
4747
}
4848

49+
function removeStyleFromAttributes(attributes: Record<string, string>, styleName: string): Record<string, string> {
50+
const attrCopy = Object.assign({}, attributes);
51+
if (!attributes.style) {
52+
return attrCopy;
53+
}
54+
55+
const map = styleStringToStyleMap(attributes.style);
56+
map.delete(styleName);
57+
58+
attrCopy.style = styleMapToStyleString(map);
59+
return attrCopy;
60+
}
61+
4962
function domElementToNode(tag: MediaNodeTag, element: HTMLElement): MediaNode {
5063
const node = $createMediaNode(tag);
5164

@@ -118,7 +131,7 @@ export class MediaNode extends ElementNode {
118131

119132
getAttributes(): Record<string, string> {
120133
const self = this.getLatest();
121-
return self.__attributes;
134+
return Object.assign({}, self.__attributes);
122135
}
123136

124137
setSources(sources: MediaNodeSource[]) {
@@ -132,7 +145,7 @@ export class MediaNode extends ElementNode {
132145
}
133146

134147
setSrc(src: string): void {
135-
const attrs = Object.assign({}, this.getAttributes());
148+
const attrs = this.getAttributes();
136149
if (this.__tag ==='object') {
137150
attrs.data = src;
138151
} else {
@@ -142,11 +155,13 @@ export class MediaNode extends ElementNode {
142155
}
143156

144157
setWidthAndHeight(width: string, height: string): void {
145-
const attrs = Object.assign(
146-
{},
158+
let attrs: Record<string, string> = Object.assign(
147159
this.getAttributes(),
148160
{width, height},
149161
);
162+
163+
attrs = removeStyleFromAttributes(attrs, 'width');
164+
attrs = removeStyleFromAttributes(attrs, 'height');
150165
this.setAttributes(attrs);
151166
}
152167

@@ -185,8 +200,8 @@ export class MediaNode extends ElementNode {
185200
return;
186201
}
187202

188-
const attrs = Object.assign({}, this.getAttributes(), {height});
189-
this.setAttributes(attrs);
203+
const attrs = Object.assign(this.getAttributes(), {height});
204+
this.setAttributes(removeStyleFromAttributes(attrs, 'height'));
190205
}
191206

192207
getHeight(): number {
@@ -195,8 +210,9 @@ export class MediaNode extends ElementNode {
195210
}
196211

197212
setWidth(width: number): void {
198-
const attrs = Object.assign({}, this.getAttributes(), {width});
199-
this.setAttributes(attrs);
213+
const existingAttrs = this.getAttributes();
214+
const attrs: Record<string, string> = Object.assign(existingAttrs, {width});
215+
this.setAttributes(removeStyleFromAttributes(attrs, 'width'));
200216
}
201217

202218
getWidth(): number {

‎resources/js/wysiwyg/lexical/rich-text/__tests__/unit/LexicalMediaNode.test.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
import {createTestContext} from "lexical/__tests__/utils";
2+
import {$createMediaNode} from "@lexical/rich-text/LexicalMediaNode";
3+
4+
5+
describe('LexicalMediaNode', () => {
6+
7+
test('setWidth/setHeight/setWidthAndHeight functions remove relevant styles', () => {
8+
const {editor} = createTestContext();
9+
editor.updateAndCommit(() => {
10+
const mediaMode = $createMediaNode('video');
11+
const defaultStyles = {style: 'width:20px;height:40px;color:red'};
12+
13+
mediaMode.setAttributes(defaultStyles);
14+
mediaMode.setWidth(60);
15+
expect(mediaMode.getWidth()).toBe(60);
16+
expect(mediaMode.getAttributes().style).toBe('height:40px;color:red');
17+
18+
mediaMode.setAttributes(defaultStyles);
19+
mediaMode.setHeight(77);
20+
expect(mediaMode.getHeight()).toBe(77);
21+
expect(mediaMode.getAttributes().style).toBe('width:20px;color:red');
22+
23+
mediaMode.setAttributes(defaultStyles);
24+
mediaMode.setWidthAndHeight('6', '7');
25+
expect(mediaMode.getWidth()).toBe(6);
26+
expect(mediaMode.getHeight()).toBe(7);
27+
expect(mediaMode.getAttributes().style).toBe('color:red');
28+
});
29+
});
30+
31+
});

‎resources/js/wysiwyg/ui/framework/helpers/node-resizer.ts‎

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ function isNodeWithSize(node: LexicalNode): node is NodeHasSize&LexicalNode {
1313
class NodeResizer {
1414
protected context: EditorUiContext;
1515
protected resizerDOM: HTMLElement|null = null;
16-
protected targetDOM: HTMLElement|null = null;
16+
protected targetNode: LexicalNode|null = null;
1717
protected scrollContainer: HTMLElement;
1818

1919
protected mouseTracker: MouseDragTracker|null = null;
@@ -38,20 +38,27 @@ class NodeResizer {
3838

3939
if (nodes.length === 1 && isNodeWithSize(nodes[0])) {
4040
const node = nodes[0];
41-
const nodeKey = node.getKey();
42-
let nodeDOM = this.context.editor.getElementByKey(nodeKey);
43-
44-
if (nodeDOM && nodeDOM.nodeName === 'SPAN') {
45-
nodeDOM = nodeDOM.firstElementChild as HTMLElement;
46-
}
41+
let nodeDOM = this.getTargetDOM(node)
4742

4843
if (nodeDOM) {
4944
this.showForNode(node, nodeDOM);
5045
}
5146
}
5247
}
5348

54-
onTargetDOMLoad(): void {
49+
protected getTargetDOM(targetNode: LexicalNode|null): HTMLElement|null {
50+
if (targetNode == null) {
51+
return null;
52+
}
53+
54+
let nodeDOM = this.context.editor.getElementByKey(targetNode.__key)
55+
if (nodeDOM && nodeDOM.nodeName === 'SPAN') {
56+
nodeDOM = nodeDOM.firstElementChild as HTMLElement;
57+
}
58+
return nodeDOM;
59+
}
60+
61+
protected onTargetDOMLoad(): void {
5562
this.updateResizerPosition();
5663
}
5764

@@ -62,7 +69,7 @@ class NodeResizer {
6269

6370
protected showForNode(node: NodeHasSize&LexicalNode, targetDOM: HTMLElement) {
6471
this.resizerDOM = this.buildDOM();
65-
this.targetDOM = targetDOM;
72+
this.targetNode = node;
6673

6774
let ghost = el('span', {class: 'editor-node-resizer-ghost'});
6875
if ($isImageNode(node)) {
@@ -83,12 +90,13 @@ class NodeResizer {
8390
}
8491

8592
protected updateResizerPosition() {
86-
if (!this.resizerDOM || !this.targetDOM) {
93+
const targetDOM = this.getTargetDOM(this.targetNode);
94+
if (!this.resizerDOM || !targetDOM) {
8795
return;
8896
}
8997

9098
const scrollAreaRect = this.scrollContainer.getBoundingClientRect();
91-
const nodeRect = this.targetDOM.getBoundingClientRect();
99+
const nodeRect = targetDOM.getBoundingClientRect();
92100
const top = nodeRect.top - (scrollAreaRect.top - this.scrollContainer.scrollTop);
93101
const left = nodeRect.left - scrollAreaRect.left;
94102

@@ -110,7 +118,7 @@ class NodeResizer {
110118
protected hide() {
111119
this.mouseTracker?.teardown();
112120
this.resizerDOM?.remove();
113-
this.targetDOM = null;
121+
this.targetNode = null;
114122
this.activeSelection = '';
115123
this.loadAbortController.abort();
116124
}
@@ -126,7 +134,7 @@ class NodeResizer {
126134
}, handleElems);
127135
}
128136

129-
setupTracker(container: HTMLElement, node: NodeHasSize, nodeDOM: HTMLElement): MouseDragTracker {
137+
setupTracker(container: HTMLElement, node: NodeHasSize&LexicalNode, nodeDOM: HTMLElement): MouseDragTracker {
130138
let startingWidth: number = 0;
131139
let startingHeight: number = 0;
132140
let startingRatio: number = 0;
@@ -179,10 +187,13 @@ class NodeResizer {
179187
_this.context.editor.update(() => {
180188
node.setWidth(size.width);
181189
node.setHeight(hasHeight ? size.height : 0);
182-
_this.context.manager.triggerLayoutUpdate();
183-
requestAnimationFrame(() => {
184-
_this.updateResizerPosition();
185-
})
190+
}, {
191+
onUpdate: () => {
192+
requestAnimationFrame(() => {
193+
_this.context.manager.triggerLayoutUpdate();
194+
_this.updateResizerPosition();
195+
});
196+
}
186197
});
187198
_this.resizerDOM?.classList.remove('active');
188199
}

‎resources/js/wysiwyg/utils/dom.ts‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,19 @@ export type StyleMap = Map<string, string>;
5252
/**
5353
* Creates a map from an element's styles.
5454
* Uses direct attribute value string handling since attempting to iterate
55-
* over .style will expand out any shorthand properties (like 'padding') making
55+
* over .style will expand out any shorthand properties (like 'padding')
5656
* rather than being representative of the actual properties set.
5757
*/
5858
export function extractStyleMapFromElement(element: HTMLElement): StyleMap {
59-
const map: StyleMap = new Map();
6059
const styleText= element.getAttribute('style') || '';
60+
return styleStringToStyleMap(styleText);
61+
}
62+
63+
/**
64+
* Convert string-formatted styles into a StyleMap.
65+
*/
66+
export function styleStringToStyleMap(styleText: string): StyleMap {
67+
const map: StyleMap = new Map();
6168

6269
const rules = styleText.split(';');
6370
for (const rule of rules) {
@@ -72,6 +79,17 @@ export function extractStyleMapFromElement(element: HTMLElement): StyleMap {
7279
return map;
7380
}
7481

82+
/**
83+
* Convert a StyleMap into inline string style text.
84+
*/
85+
export function styleMapToStyleString(map: StyleMap): string {
86+
const parts = [];
87+
for (const [style, value] of map.entries()) {
88+
parts.push(`${style}:${value}`);
89+
}
90+
return parts.join(';');
91+
}
92+
7593
export function setOrRemoveAttribute(element: HTMLElement, name: string, value: string|null|undefined) {
7694
if (value) {
7795
element.setAttribute(name, value);

‎resources/sass/_editor.scss‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,7 @@ body.editor-is-fullscreen {
454454
.editor-media-wrap {
455455
display: inline-block;
456456
cursor: not-allowed;
457-
iframe {
457+
iframe, video {
458458
pointer-events: none;
459459
}
460460
&.align-left {

0 commit comments

Comments
 (0)