Skip to content
Snippets Groups Projects
Unverified Commit 09ab44a3 authored by Kamil Mielnik's avatar Kamil Mielnik Committed by GitHub
Browse files

Fix - `useIsTruncated` gives incorrect results (#45625)

* Get rid of tolerance

* Use range rect and element rect instead of clientWidth/clientHeight/scrollWidth/scrollHeight

* Fix types

* Mock Range.prototype.getBoundingClientRect

* Mock getBoundingClientRect in unit tests
parent 00d574e5
No related branches found
No related tags found
No related merge requests found
......@@ -39,9 +39,7 @@ export const CollectionBreadcrumbsWithTooltip = ({
: collections;
const justOneShown = shownCollections.length === 1;
const { areAnyTruncated, ref } = useAreAnyTruncated<HTMLDivElement>({
tolerance: 1,
});
const { areAnyTruncated, ref } = useAreAnyTruncated<HTMLDivElement>();
const initialEllipsisRef = useRef<HTMLDivElement | null>(null);
const [
......
......@@ -149,27 +149,24 @@ describe("PinnedItemCard", () => {
});
describe("description", () => {
const originalScrollWidth = Object.getOwnPropertyDescriptor(
HTMLElement.prototype,
"scrollWidth",
);
const getBoundingClientRect = HTMLElement.prototype.getBoundingClientRect;
const rangeGetBoundingClientRect = Range.prototype.getBoundingClientRect;
beforeAll(() => {
// emulate ellipsis
Object.defineProperty(HTMLElement.prototype, "scrollWidth", {
configurable: true,
value: 100,
});
// Mock return values so that getIsTruncated can kick in
HTMLElement.prototype.getBoundingClientRect = jest
.fn()
.mockReturnValue({ height: 1, width: 1 });
Range.prototype.getBoundingClientRect = jest
.fn()
.mockReturnValue({ height: 1, width: 2 });
});
afterAll(() => {
if (originalScrollWidth) {
Object.defineProperty(
HTMLElement.prototype,
"scrollWidth",
originalScrollWidth,
);
}
HTMLElement.prototype.getBoundingClientRect = getBoundingClientRect;
Range.prototype.getBoundingClientRect = rangeGetBoundingClientRect;
jest.resetAllMocks();
});
it("should render description markdown as plain text", () => {
......
......@@ -43,27 +43,24 @@ describe("MarkdownPreview", () => {
});
describe("Tooltip on ellipsis", () => {
const originalScrollWidth = Object.getOwnPropertyDescriptor(
HTMLElement.prototype,
"scrollWidth",
);
const getBoundingClientRect = HTMLElement.prototype.getBoundingClientRect;
const rangeGetBoundingClientRect = Range.prototype.getBoundingClientRect;
beforeAll(() => {
// emulate ellipsis
Object.defineProperty(HTMLElement.prototype, "scrollWidth", {
configurable: true,
value: 100,
});
// Mock return values so that getIsTruncated can kick in
HTMLElement.prototype.getBoundingClientRect = jest
.fn()
.mockReturnValue({ height: 1, width: 1 });
Range.prototype.getBoundingClientRect = jest
.fn()
.mockReturnValue({ height: 1, width: 2 });
});
afterAll(() => {
if (originalScrollWidth) {
Object.defineProperty(
HTMLElement.prototype,
"scrollWidth",
originalScrollWidth,
);
}
HTMLElement.prototype.getBoundingClientRect = getBoundingClientRect;
Range.prototype.getBoundingClientRect = rangeGetBoundingClientRect;
jest.resetAllMocks();
});
it("should show tooltip with markdown formatting on hover when text is truncated", async () => {
......
......@@ -5,13 +5,10 @@ import resizeObserver from "metabase/lib/resize-observer";
type UseIsTruncatedProps = {
disabled?: boolean;
/** To avoid rounding errors, we can require that the truncation is at least a certain number of pixels */
tolerance?: number;
};
export const useIsTruncated = <E extends Element>({
disabled = false,
tolerance = 0,
}: UseIsTruncatedProps = {}) => {
const ref = useRef<E | null>(null);
const [isTruncated, setIsTruncated] = useState(false);
......@@ -24,7 +21,7 @@ export const useIsTruncated = <E extends Element>({
}
const handleResize = () => {
setIsTruncated(getIsTruncated(element, tolerance));
setIsTruncated(getIsTruncated(element));
};
handleResize();
......@@ -33,21 +30,24 @@ export const useIsTruncated = <E extends Element>({
return () => {
resizeObserver.unsubscribe(element, handleResize);
};
}, [disabled, tolerance]);
}, [disabled]);
return { isTruncated, ref };
};
const getIsTruncated = (element: Element, tolerance: number): boolean => {
const getIsTruncated = (element: Element): boolean => {
const range = document.createRange();
range.selectNodeContents(element);
const elementRect = element.getBoundingClientRect();
const rangeRect = range.getBoundingClientRect();
return (
element.scrollHeight > element.clientHeight + tolerance ||
element.scrollWidth > element.clientWidth + tolerance
rangeRect.height > elementRect.height || rangeRect.width > elementRect.width
);
};
export const useAreAnyTruncated = <E extends Element>({
disabled = false,
tolerance = 0,
}: UseIsTruncatedProps = {}) => {
const ref = useRef(new Map<string, E>());
const [truncationStatusByKey, setTruncationStatusByKey] = useState<
......@@ -64,7 +64,7 @@ export const useAreAnyTruncated = <E extends Element>({
[...elementsMap.entries()].forEach(([elementKey, element]) => {
const handleResize = () => {
const isTruncated = getIsTruncated(element, tolerance);
const isTruncated = getIsTruncated(element);
setTruncationStatusByKey(statuses => {
const newStatuses = new Map(statuses);
newStatuses.set(elementKey, isTruncated);
......@@ -81,7 +81,7 @@ export const useAreAnyTruncated = <E extends Element>({
return () => {
unsubscribeFns.forEach(fn => fn());
};
}, [disabled, tolerance]);
}, [disabled]);
const areAnyTruncated = [...truncationStatusByKey.values()].some(Boolean);
return { areAnyTruncated, ref };
......
......@@ -27,27 +27,24 @@ function setup({ description }: { description?: string } = {}) {
describe("StaticSkeleton", () => {
describe("description", () => {
const originalScrollWidth = Object.getOwnPropertyDescriptor(
HTMLElement.prototype,
"scrollWidth",
);
const getBoundingClientRect = HTMLElement.prototype.getBoundingClientRect;
const rangeGetBoundingClientRect = Range.prototype.getBoundingClientRect;
beforeAll(() => {
// emulate ellipsis
Object.defineProperty(HTMLElement.prototype, "scrollWidth", {
configurable: true,
value: 100,
});
// Mock return values so that getIsTruncated can kick in
HTMLElement.prototype.getBoundingClientRect = jest
.fn()
.mockReturnValue({ height: 1, width: 1 });
Range.prototype.getBoundingClientRect = jest
.fn()
.mockReturnValue({ height: 1, width: 2 });
});
afterAll(() => {
if (originalScrollWidth) {
Object.defineProperty(
HTMLElement.prototype,
"scrollWidth",
originalScrollWidth,
);
}
HTMLElement.prototype.getBoundingClientRect = getBoundingClientRect;
Range.prototype.getBoundingClientRect = rangeGetBoundingClientRect;
jest.resetAllMocks();
});
it("should render description markdown as plain text", () => {
......
......@@ -31,3 +31,13 @@ if (process.env["DISABLE_LOGGING"] || process.env["DISABLE_LOGGING_FRONTEND"]) {
// (hacky fix)
global.TextEncoder = TextEncoder;
global.TextDecoder = TextDecoder;
// https://github.com/jsdom/jsdom/issues/3002
Range.prototype.getBoundingClientRect = () => ({
bottom: 0,
height: 0,
left: 0,
right: 0,
top: 0,
width: 0,
});
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment