fix(editor): load the float responsive rule + test applyAlignment (#145 review)
Review of #157 (Request changes) caught two blockers: 1. DEAD responsive CSS: the `@media (max-width:600px)` float-reset was added to `image-resize.module.css`, which is imported NOWHERE — the image container's classes come from `common/node-resize.module.css` (via buildResizeClasses). So on mobile a floated image kept its px width + float and crushed the text, exactly the failure the rule promised to prevent. Moved the rule to `common/node-resize.module.css` (the module actually imported by the resize node views); its `:global([data-image-align=...])` selectors are data-attr based, so they work unchanged. Reverted the dead addition from the (pre-existing, orphaned) image-resize.module.css. 2. `applyAlignment` was untested. Exported it and added `image.spec.ts` (vitest/ jsdom) covering all five align values, the data-image-align mirror, and the floatLeft -> left reset-then-apply (the guard against a leaked float). Switched the float writes to the canonical CSSOM `cssFloat` property (portable: browsers + jsdom; behavior identical to the `.float` alias). editor-ext build + client tsc clean; 6 image.spec tests green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -73,3 +73,18 @@
|
|||||||
display: none !important;
|
display: none !important;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* Float image (#145): on narrow screens a floated image would crowd the text to
|
||||||
|
an unreadable column, so collapse it to full width and drop the float.
|
||||||
|
`!important` is required because applyAlignment sets `float`/`padding` inline,
|
||||||
|
which a normal rule cannot override. Keys off the `data-image-align` attribute
|
||||||
|
the image node view mirrors onto its container. This module is the one actually
|
||||||
|
imported by the resize node views (node-resize-handles.ts), so the rule loads. */
|
||||||
|
@media (max-width: 600px) {
|
||||||
|
:global([data-image-align="floatLeft"]),
|
||||||
|
:global([data-image-align="floatRight"]) {
|
||||||
|
float: none !important;
|
||||||
|
width: 100% !important;
|
||||||
|
padding: 0 !important;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -62,17 +62,3 @@
|
|||||||
.resizing .handleBar {
|
.resizing .handleBar {
|
||||||
background-color: light-dark(var(--mantine-color-blue-6), var(--mantine-color-blue-4));
|
background-color: light-dark(var(--mantine-color-blue-6), var(--mantine-color-blue-4));
|
||||||
}
|
}
|
||||||
|
|
||||||
/* Float image (#145): on narrow screens a floated image would crowd the text to
|
|
||||||
an unreadable column, so collapse it to full width and drop the float.
|
|
||||||
`!important` is required because applyAlignment sets `float`/`padding` inline,
|
|
||||||
which a normal rule cannot override. Keys off the data-image-align attribute
|
|
||||||
the node view mirrors onto the container. */
|
|
||||||
@media (max-width: 600px) {
|
|
||||||
:global([data-image-align="floatLeft"]),
|
|
||||||
:global([data-image-align="floatRight"]) {
|
|
||||||
float: none !important;
|
|
||||||
width: 100% !important;
|
|
||||||
padding: 0 !important;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|||||||
67
packages/editor-ext/src/lib/image/image.spec.ts
Normal file
67
packages/editor-ext/src/lib/image/image.spec.ts
Normal file
@@ -0,0 +1,67 @@
|
|||||||
|
import { describe, it, expect, beforeEach } from "vitest";
|
||||||
|
import { applyAlignment } from "./image";
|
||||||
|
|
||||||
|
// applyAlignment is a pure DOM mutation: it sets the float / padding /
|
||||||
|
// justify-content / data-image-align on an image node-view container per the
|
||||||
|
// resolved `align`. Tested directly (issue #145 review) since the five-way
|
||||||
|
// branch, the reset-then-apply guard, and the data-image-align mirror (which the
|
||||||
|
// responsive @media rule keys off) are otherwise uncovered.
|
||||||
|
|
||||||
|
describe("applyAlignment", () => {
|
||||||
|
let el: HTMLElement;
|
||||||
|
beforeEach(() => {
|
||||||
|
el = document.createElement("div");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("floatLeft -> float:left + right padding, mirrored on data-image-align", () => {
|
||||||
|
applyAlignment(el, "floatLeft");
|
||||||
|
expect(el.style.cssFloat).toBe("left");
|
||||||
|
expect(el.style.padding).toBe("0px 10px 0px 0px");
|
||||||
|
expect(el.dataset.imageAlign).toBe("floatLeft");
|
||||||
|
expect(el.style.justifyContent).toBe("flex-start");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("floatRight -> float:right + left padding", () => {
|
||||||
|
applyAlignment(el, "floatRight");
|
||||||
|
expect(el.style.cssFloat).toBe("right");
|
||||||
|
expect(el.style.padding).toBe("0px 0px 0px 10px");
|
||||||
|
expect(el.dataset.imageAlign).toBe("floatRight");
|
||||||
|
expect(el.style.justifyContent).toBe("flex-end");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("left -> justify flex-start, no float", () => {
|
||||||
|
applyAlignment(el, "left");
|
||||||
|
expect(el.style.justifyContent).toBe("flex-start");
|
||||||
|
expect(el.style.cssFloat).toBe("");
|
||||||
|
expect(el.style.padding).toBe("");
|
||||||
|
expect(el.dataset.imageAlign).toBe("left");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("right -> justify flex-end, no float", () => {
|
||||||
|
applyAlignment(el, "right");
|
||||||
|
expect(el.style.justifyContent).toBe("flex-end");
|
||||||
|
expect(el.style.cssFloat).toBe("");
|
||||||
|
expect(el.dataset.imageAlign).toBe("right");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("center (default) -> justify center, no float", () => {
|
||||||
|
applyAlignment(el, "center");
|
||||||
|
expect(el.style.justifyContent).toBe("center");
|
||||||
|
expect(el.style.cssFloat).toBe("");
|
||||||
|
expect(el.style.padding).toBe("");
|
||||||
|
expect(el.dataset.imageAlign).toBe("center");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("clears a previous float when switching floatLeft -> left (reset-then-apply)", () => {
|
||||||
|
applyAlignment(el, "floatLeft");
|
||||||
|
expect(el.style.cssFloat).toBe("left");
|
||||||
|
expect(el.style.padding).toBe("0px 10px 0px 0px");
|
||||||
|
// Switching to a block alignment must drop the float and its padding, not
|
||||||
|
// leak them (the bug the reset guard prevents).
|
||||||
|
applyAlignment(el, "left");
|
||||||
|
expect(el.style.cssFloat).toBe("");
|
||||||
|
expect(el.style.padding).toBe("");
|
||||||
|
expect(el.dataset.imageAlign).toBe("left");
|
||||||
|
expect(el.style.justifyContent).toBe("flex-start");
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -376,10 +376,10 @@ export const TiptapImage = Image.extend<ImageOptions>({
|
|||||||
},
|
},
|
||||||
});
|
});
|
||||||
|
|
||||||
function applyAlignment(container: HTMLElement, align: string) {
|
export function applyAlignment(container: HTMLElement, align: string) {
|
||||||
// Reset the float-mode styles first so toggling between any two modes is clean
|
// Reset the float-mode styles first so toggling between any two modes is clean
|
||||||
// (a previous float must not leak into a later left/center/right).
|
// (a previous float must not leak into a later left/center/right).
|
||||||
container.style.float = "";
|
container.style.cssFloat = "";
|
||||||
container.style.padding = "";
|
container.style.padding = "";
|
||||||
// Mirror the resolved alignment onto the CONTAINER as a data attribute so the
|
// Mirror the resolved alignment onto the CONTAINER as a data attribute so the
|
||||||
// responsive stylesheet can neutralize the float on small screens (an inline
|
// responsive stylesheet can neutralize the float on small screens (an inline
|
||||||
@@ -389,11 +389,11 @@ function applyAlignment(container: HTMLElement, align: string) {
|
|||||||
if (align === "floatLeft") {
|
if (align === "floatLeft") {
|
||||||
// Real text wrap: the (shrink-to-fit) container floats left, text flows on
|
// Real text wrap: the (shrink-to-fit) container floats left, text flows on
|
||||||
// its right. The inner <img> already carries max-width:100%.
|
// its right. The inner <img> already carries max-width:100%.
|
||||||
container.style.float = "left";
|
container.style.cssFloat = "left";
|
||||||
container.style.padding = "0 10px 0 0";
|
container.style.padding = "0 10px 0 0";
|
||||||
container.style.justifyContent = "flex-start";
|
container.style.justifyContent = "flex-start";
|
||||||
} else if (align === "floatRight") {
|
} else if (align === "floatRight") {
|
||||||
container.style.float = "right";
|
container.style.cssFloat = "right";
|
||||||
container.style.padding = "0 0 0 10px";
|
container.style.padding = "0 0 0 10px";
|
||||||
container.style.justifyContent = "flex-end";
|
container.style.justifyContent = "flex-end";
|
||||||
} else if (align === "left") {
|
} else if (align === "left") {
|
||||||
|
|||||||
Reference in New Issue
Block a user