Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const mentionStyle = `
color: var(--pill-generic-label-alt-default);
cursor: pointer;
display: inline-block;
font: var(--global-font-static-comp-regular-m);
font: inherit;
min-height: 20px;
padding: 0px var(--global-space-comp-s);
Comment thread
tamas-sage marked this conversation as resolved.
margin: 0px var(--global-space-comp-xs);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,65 @@ describe("StyledSpanNode", () => {
});
});

test("should not set inline font size or line height on paragraph nodes", () => {
editor?.update(() => {
const node = StyledSpanNode.createFromOption("paragraph", "Body text");
const domElement = node.createDOM(staticConfig);

// eslint-disable-next-line jest-dom/prefer-to-have-style

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

question: I'm guessing we're having to ignore the linter here due to some weirdness of Lexical?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, exactly. It is not a Lexical bug, we are intentionally checking direct inline style values on element.style (including empty string cases), and toHaveStyle is less precise for that specific assertion.

expect(domElement.style.fontSize).toBe("");
// eslint-disable-next-line jest-dom/prefer-to-have-style
expect(domElement.style.lineHeight).toBe("");
});
});

test("should keep bold on a paragraph node without setting an inline font size", () => {
editor?.update(() => {
const node = StyledSpanNode.createFromOption("paragraph", "Body text");
node.toggleFormat("bold");

const domElement = node.createDOM(staticConfig);

// eslint-disable-next-line jest-dom/prefer-to-have-style
expect(domElement.style.fontWeight).toBe("700");
// eslint-disable-next-line jest-dom/prefer-to-have-style
expect(domElement.style.fontSize).toBe("");
});
});

test("should apply inline styles when a paragraph node becomes a title", () => {
editor?.update(() => {
const prevNode = StyledSpanNode.createFromOption("paragraph", "Text");
const currentNode = StyledSpanNode.createFromOption("title", "Text");
const domElement = prevNode.createDOM(staticConfig);

currentNode.updateDOM(prevNode, domElement, staticConfig);

// eslint-disable-next-line jest-dom/prefer-to-have-style
expect(domElement.style.fontSize).toBe("24px");
// eslint-disable-next-line jest-dom/prefer-to-have-style
expect(domElement.style.lineHeight).toBe("30px");
});
});

test("should clear inline styles when a title node becomes a paragraph", () => {
editor?.update(() => {
const prevNode = StyledSpanNode.createFromOption("title", "Text");
const currentNode = StyledSpanNode.createFromOption(
"paragraph",
"Text",
);
const domElement = prevNode.createDOM(staticConfig);

currentNode.updateDOM(prevNode, domElement, staticConfig);

// eslint-disable-next-line jest-dom/prefer-to-have-style
expect(domElement.style.fontSize).toBe("");
// eslint-disable-next-line jest-dom/prefer-to-have-style
expect(domElement.style.lineHeight).toBe("");
});
});

test("should update DOM when styles change", () => {
editor?.update(() => {
const prevNode = new StyledSpanNode("Test", "400", "14px", "21px");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,21 @@ export class StyledSpanNode extends TextNode {
writable.__lineHeight = lineHeight;
}

/**
* Whether this node uses the paragraph preset. Paragraph font size and line height
* are inherited from the editor so they can scale with its `size` prop, so they are
* deliberately not written as inline styles.
* @returns true when the node matches the paragraph preset
Comment thread
tamas-sage marked this conversation as resolved.
*/
usesParagraphPreset(): boolean {
const { weight, size, lineHeight } = typographyMap.paragraph;
return (
this.__fontWeight === weight &&
this.__fontSize === size &&
this.__lineHeight === lineHeight
);
}

/**
* Determine the typography key based on the current styles
* @returns the variant of this styled span
Expand Down Expand Up @@ -254,8 +269,10 @@ export class StyledSpanNode extends TextNode {
const dom = super.createDOM(_config);
const format = this.__format;
dom.style.fontWeight = format & IS_BOLD ? "700" : this.__fontWeight;
dom.style.fontSize = this.__fontSize;
dom.style.lineHeight = this.__lineHeight;
if (!this.usesParagraphPreset()) {
dom.style.fontSize = this.__fontSize;
dom.style.lineHeight = this.__lineHeight;
}
if (format & IS_ITALIC) {
dom.style.fontStyle = "italic";
}
Expand All @@ -278,12 +295,20 @@ export class StyledSpanNode extends TextNode {
dom.style.fontWeight = nextEffectiveWeight;
updated = true;
}
if (this.__fontSize !== prevNode.__fontSize) {
dom.style.fontSize = this.__fontSize;
const isParagraph = this.usesParagraphPreset();

if (
this.__fontSize !== prevNode.__fontSize ||
isParagraph !== prevNode.usesParagraphPreset()
) {
dom.style.fontSize = isParagraph ? "" : this.__fontSize;
updated = true;
}
if (this.__lineHeight !== prevNode.__lineHeight) {
dom.style.lineHeight = this.__lineHeight;
if (
this.__lineHeight !== prevNode.__lineHeight ||
isParagraph !== prevNode.usesParagraphPreset()
) {
dom.style.lineHeight = isParagraph ? "" : this.__lineHeight;
updated = true;
}

Expand Down
1 change: 1 addition & 0 deletions src/components/text-editor/text-editor.component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,7 @@ export const TextEditor = forwardRef<TextEditorHandle, TextEditorProps>(
<StyledTextEditor
data-role={`${namespace}-editor`}
error={hasError}
size={actualSize}
>
<RichTextPlugin
contentEditable={
Expand Down
9 changes: 8 additions & 1 deletion src/components/text-editor/text-editor.style.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,28 +3,35 @@ import { margin } from "styled-system";

const sizeMap = {
small: {
paragraphFont: "var(--global-font-static-comp-regular-s)",
contentMargin: "var(--global-space-comp-xs)",
validationGap: "var(--global-space-comp-xs)",
headerPadding: "var(--global-space-comp-s)",
footerPadding: "var(--global-space-comp-s)",
},
medium: {
paragraphFont: "var(--global-font-static-comp-regular-m)",
contentMargin: "var(--global-space-comp-s)",
validationGap: "var(--global-space-comp-s)",
headerPadding: "var(--global-space-comp-m)",
footerPadding: "var(--global-space-comp-m)",
},
large: {
paragraphFont: "var(--global-font-static-comp-regular-l)",
contentMargin: "var(--global-space-comp-m)",
validationGap: "var(--global-space-comp-s)",
headerPadding: "var(--global-space-comp-l)",
footerPadding: "var(--global-space-comp-l)",
},
};

export const StyledTextEditor = styled.div<{ error?: boolean }>`
export const StyledTextEditor = styled.div<{
error?: boolean;
size: "small" | "medium" | "large";
}>`
position: relative;
box-sizing: border-box;
font: ${({ size }) => sizeMap[size].paragraphFont};
${({ error }) =>
error &&
css`
Expand Down
Loading