From c8f3c15ca9619a7f20e81d6c26954d3cee3dbf36 Mon Sep 17 00:00:00 2001 From: lskramarov Date: Wed, 1 Jul 2026 16:25:57 +0300 Subject: [PATCH] fix(title): don't show tooltip for imperceptible sub-pixel clip (#DS-4292) --- .../components/title/title.directive.spec.ts | 202 ++++++++++++++++++ packages/components/title/title.directive.ts | 42 +++- 2 files changed, 242 insertions(+), 2 deletions(-) diff --git a/packages/components/title/title.directive.spec.ts b/packages/components/title/title.directive.spec.ts index 9a10604d0..16dffa630 100644 --- a/packages/components/title/title.directive.spec.ts +++ b/packages/components/title/title.directive.spec.ts @@ -199,6 +199,208 @@ describe('KbqTitleDirective', () => { expect(directive.isOverflown).toBe(true); }); + + it('should NOT be overflown for a sub-pixel clip (text-overflow: clip)', () => { + const { debugElement } = createComponent(SimpleTitleComponent); + const directive = getTitleDirective(debugElement); + // JSDOM scrollWidth === 0 → enters the special-case branch; hasOnlyText === true → wrapper-span path. + // With `clip` a <1px overflow is invisible, so it must not be reported as truncation. + const cssSpy = jest + .spyOn(window, 'getComputedStyle') + .mockReturnValue({ textOverflow: 'clip' } as CSSStyleDeclaration); + const rectSpy = jest + .spyOn(Element.prototype, 'getBoundingClientRect') + .mockReturnValueOnce({ width: 124, height: 20, top: 0, left: 0, right: 124, bottom: 20 } as DOMRect) + .mockReturnValueOnce({ + width: 124.4, + height: 20, + top: 0, + left: 0, + right: 124.4, + bottom: 20 + } as DOMRect); + + expect(directive.isOverflown).toBe(false); + + rectSpy.mockRestore(); + cssSpy.mockRestore(); + }); + + it('should be overflown for a sub-pixel overflow when text-overflow is ellipsis', () => { + const { debugElement } = createComponent(SimpleTitleComponent); + const directive = getTitleDirective(debugElement); + // With `ellipsis` even a sub-pixel overflow drops the trailing glyph for `…`, so the text IS truncated. + const cssSpy = jest + .spyOn(window, 'getComputedStyle') + .mockReturnValue({ textOverflow: 'ellipsis' } as CSSStyleDeclaration); + const rectSpy = jest + .spyOn(Element.prototype, 'getBoundingClientRect') + .mockReturnValueOnce({ width: 124, height: 20, top: 0, left: 0, right: 124, bottom: 20 } as DOMRect) + .mockReturnValueOnce({ + width: 124.4, + height: 20, + top: 0, + left: 0, + right: 124.4, + bottom: 20 + } as DOMRect); + + expect(directive.isOverflown).toBe(true); + + rectSpy.mockRestore(); + cssSpy.mockRestore(); + }); + + it('should be overflown for a >= 1px clip even without ellipsis', () => { + const { debugElement } = createComponent(SimpleTitleComponent); + const directive = getTitleDirective(debugElement); + const cssSpy = jest + .spyOn(window, 'getComputedStyle') + .mockReturnValue({ textOverflow: 'clip' } as CSSStyleDeclaration); + const rectSpy = jest + .spyOn(Element.prototype, 'getBoundingClientRect') + .mockReturnValueOnce({ width: 124, height: 20, top: 0, left: 0, right: 124, bottom: 20 } as DOMRect) + .mockReturnValueOnce({ width: 130, height: 20, top: 0, left: 0, right: 130, bottom: 20 } as DOMRect); + + expect(directive.isOverflown).toBe(true); + + rectSpy.mockRestore(); + cssSpy.mockRestore(); + }); + + it('should NOT be overflown for a sub-pixel clip (element-child, text-overflow: clip)', () => { + const { debugElement } = createComponent(WithRefsTitleComponent); + const directive = getTitleDirective(debugElement); + const containerEl = debugElement.query(By.css('.container-el')).nativeElement; + const textEl = debugElement.query(By.css('.text-el')).nativeElement; + // scrollWidth === 0 (JSDOM) → enters the branch; hasOnlyText === false → parent/child rect comparison. + const cssSpy = jest + .spyOn(window, 'getComputedStyle') + .mockReturnValue({ textOverflow: 'clip' } as CSSStyleDeclaration); + + jest.spyOn(containerEl, 'getBoundingClientRect').mockReturnValue({ + width: 100, + height: 20, + top: 0, + left: 0, + right: 100, + bottom: 20 + } as DOMRect); + jest.spyOn(textEl, 'getBoundingClientRect').mockReturnValue({ + width: 100.4, + height: 20, + top: 0, + left: 0, + right: 100.4, + bottom: 20 + } as DOMRect); + + expect(directive.isOverflown).toBe(false); + + cssSpy.mockRestore(); + }); + + it('should be overflown for a sub-pixel overflow when text-overflow is ellipsis (element-child)', () => { + const { debugElement } = createComponent(WithRefsTitleComponent); + const directive = getTitleDirective(debugElement); + const containerEl = debugElement.query(By.css('.container-el')).nativeElement; + const textEl = debugElement.query(By.css('.text-el')).nativeElement; + const cssSpy = jest + .spyOn(window, 'getComputedStyle') + .mockReturnValue({ textOverflow: 'ellipsis' } as CSSStyleDeclaration); + + jest.spyOn(containerEl, 'getBoundingClientRect').mockReturnValue({ + width: 100, + height: 20, + top: 0, + left: 0, + right: 100, + bottom: 20 + } as DOMRect); + jest.spyOn(textEl, 'getBoundingClientRect').mockReturnValue({ + width: 100.4, + height: 20, + top: 0, + left: 0, + right: 100.4, + bottom: 20 + } as DOMRect); + + expect(directive.isOverflown).toBe(true); + + cssSpy.mockRestore(); + }); + + it('should be overflown for a sub-pixel overflow when the ellipsis is on the container, not the child (tree-option case)', () => { + const { debugElement } = createComponent(WithRefsTitleComponent); + const directive = getTitleDirective(debugElement); + const containerEl = debugElement.query(By.css('.container-el')).nativeElement; + const textEl = debugElement.query(By.css('.text-el')).nativeElement; + // Mirrors kbq-tree-option: `text-overflow: ellipsis` lives on the wrapping #kbqTitleContainer + // (the parent), while the measured #kbqTitleText child keeps the default `clip`. A sub-pixel + // overflow still renders a visible `…` on the container, so it MUST be reported as truncation. + const cssSpy = jest + .spyOn(window, 'getComputedStyle') + .mockImplementation( + (element: Element) => + ({ textOverflow: element === containerEl ? 'ellipsis' : 'clip' }) as CSSStyleDeclaration + ); + + jest.spyOn(containerEl, 'getBoundingClientRect').mockReturnValue({ + width: 100, + height: 20, + top: 0, + left: 0, + right: 100, + bottom: 20 + } as DOMRect); + jest.spyOn(textEl, 'getBoundingClientRect').mockReturnValue({ + width: 100.4, + height: 20, + top: 0, + left: 0, + right: 100.4, + bottom: 20 + } as DOMRect); + + expect(directive.isOverflown).toBe(true); + + cssSpy.mockRestore(); + }); + + it('should be overflown for a clip whose widths straddle the rounding boundary (124 vs 124.5)', () => { + const { debugElement } = createComponent(WithRefsTitleComponent); + const directive = getTitleDirective(debugElement); + const containerEl = debugElement.query(By.css('.container-el')).nativeElement; + const textEl = debugElement.query(By.css('.text-el')).nativeElement; + // The other clip tests use a 0.4 fraction that rounds DOWN (124.4 -> 124 == 124 -> not overflown). + // Here the widths round to different integers (124 vs 125), i.e. a whole visible pixel of clip, + // so it MUST be reported. Pins the Math.round boundary of isWidthOverflown. + const cssSpy = jest + .spyOn(window, 'getComputedStyle') + .mockReturnValue({ textOverflow: 'clip' } as CSSStyleDeclaration); + + jest.spyOn(containerEl, 'getBoundingClientRect').mockReturnValue({ + width: 124, + height: 20, + top: 0, + left: 0, + right: 124, + bottom: 20 + } as DOMRect); + jest.spyOn(textEl, 'getBoundingClientRect').mockReturnValue({ + width: 124.5, + height: 20, + top: 0, + left: 0, + right: 124.5, + bottom: 20 + } as DOMRect); + + expect(directive.isOverflown).toBe(true); + + cssSpy.mockRestore(); + }); }); describe('handleElementEnter()', () => { diff --git a/packages/components/title/title.directive.ts b/packages/components/title/title.directive.ts index af182e358..efb5ceef3 100644 --- a/packages/components/title/title.directive.ts +++ b/packages/components/title/title.directive.ts @@ -13,6 +13,7 @@ import { } from '@angular/core'; import { KBQ_TITLE_TEXT_REF, + KBQ_WINDOW, kbqInjectNativeElement, KbqTitleTextRef, PopUpPlacements, @@ -51,6 +52,9 @@ export class KbqTitleDirective extends KbqTooltipTrigger implements AfterViewIni /** Host native element the directive is attached to. */ private readonly nativeElement = kbqInjectNativeElement(); + /** SSR-safe window reference used for `getComputedStyle` reads. */ + private readonly window = inject(KBQ_WINDOW); + /** Observes host content mutations to re-evaluate overflow and refresh the resolved tooltip content. */ private contentObserver = inject(ContentObserver); @@ -102,14 +106,20 @@ export class KbqTitleDirective extends KbqTooltipTrigger implements AfterViewIni wrapper.innerText = this.child.innerText; this.parent.appendChild(wrapper); - const result = this.parent.getBoundingClientRect().width < wrapper.getBoundingClientRect().width; + const result = this.isWidthOverflown( + this.parent.getBoundingClientRect().width, + wrapper.getBoundingClientRect().width + ); wrapper.remove(); return result; } - return this.parent.getBoundingClientRect().width < this.child.getBoundingClientRect().width; + return this.isWidthOverflown( + this.parent.getBoundingClientRect().width, + this.child.getBoundingClientRect().width + ); } return this.isHorizontalOverflown || this.isVerticalOverflown; @@ -125,6 +135,16 @@ export class KbqTitleDirective extends KbqTooltipTrigger implements AfterViewIni return this.parent?.offsetHeight < this.child.scrollHeight; } + /** + * Compares measured widths, treating only *visible* clipping as overflow. With `text-overflow: ellipsis` + * any positive difference counts (even a sub-pixel overflow shows `…`). With `text-overflow: clip` the + * widths are rounded to whole CSS pixels first — mirroring the integer `offsetWidth`/`scrollWidth` path — + * so an imperceptible sub-pixel clip is not treated as truncation. + * @docs-private */ + private isWidthOverflown(parentWidth: number, childWidth: number): boolean { + return this.hasEllipsis ? parentWidth < childWidth : Math.round(parentWidth) < Math.round(childWidth); + } + /** Trimmed `textContent` of the measured parent, used as the default tooltip content. */ get viewValue(): string { return (this.parent?.textContent || '').trim(); @@ -155,6 +175,24 @@ export class KbqTitleDirective extends KbqTooltipTrigger implements AfterViewIni ); } + /** + * Whether the measured text truncates with an ellipsis on either the child text element or its + * wrapping container. Only then is a sub-pixel overflow actually visible (the trailing glyph is + * replaced by `…`); with the default `text-overflow: clip` a sub-pixel clip is imperceptible, so + * it must not be reported as truncation. Both elements are checked because `text-overflow` is not + * inherited and consumers place it differently: `KbqOption`/`KbqDropdownItem` style the measured + * `child`, whereas `KbqTreeOption` styles the `parent` container that wraps the child. + * @docs-private */ + private get hasEllipsis(): boolean { + return this.elementHasEllipsis(this.child) || this.elementHasEllipsis(this.parent); + } + + /** Whether the element's computed `text-overflow` renders an ellipsis. + * @docs-private */ + private elementHasEllipsis(element: HTMLElement): boolean { + return this.window.getComputedStyle(element).textOverflow.includes('ellipsis'); + } + /** * Effective text elements used for overflow detection. Resolves to the projected `#kbqTitleText` elements, * otherwise falls back to the `KbqTitleTextRef` host's `textElement`, otherwise to the host element itself.