From c11f886c2dedac56cf9242d47268cc6b1480e56b Mon Sep 17 00:00:00 2001 From: lskramarov Date: Wed, 10 Jun 2026 23:54:54 +0300 Subject: [PATCH 1/6] fix(button): block clicks and ENTER/SPACE on disabled anchor hosts, set aria-disabled (#DS-4364) --- packages/components/button/_button-theme.scss | 14 +- .../components/button/button.component.html | 8 +- .../button/button.component.spec.ts | 71 +++++++- .../components/button/button.component.ts | 168 ++++++++++++------ packages/components/button/button.scss | 11 -- .../filter-bar/_filter-bar-theme.scss | 4 - .../public_api_guard/components/button.api.md | 18 +- 7 files changed, 198 insertions(+), 96 deletions(-) diff --git a/packages/components/button/_button-theme.scss b/packages/components/button/_button-theme.scss index 2947bcb391..407e364d0b 100644 --- a/packages/components/button/_button-theme.scss +++ b/packages/components/button/_button-theme.scss @@ -46,10 +46,6 @@ } @mixin kbq-button-theme() { - .kbq-button-overlay { - display: none; - } - .kbq-button, .kbq-button-icon { -webkit-font-smoothing: antialiased; @@ -87,7 +83,9 @@ &.kbq-button_filled, &.kbq-button_outline, &.kbq-button_transparent { - &.cdk-keyboard-focused { + // :not(.kbq-disabled): a disabled can keep DOM focus (tabindex=-1 does + // not blur it), and FocusMonitor stays attached for the whole component lifetime. + &.cdk-keyboard-focused:not(.kbq-disabled) { $focused-color: var(--kbq-states-line-focus-theme); outline: 1px solid $focused-color; border-color: $focused-color; @@ -101,10 +99,4 @@ .kbq-button-icon { @include kbq-typography-level-to-styles-css-variables(typography, text-normal-medium); } - - .kbq-button-icon.kbq-button-wrapper { - .kbq-icon { - @include kbq-css-font-variable(typography, body, line-height, ''); - } - } } diff --git a/packages/components/button/button.component.html b/packages/components/button/button.component.html index 467740ebfd..4b536380de 100644 --- a/packages/components/button/button.component.html +++ b/packages/components/button/button.component.html @@ -1,4 +1,8 @@ - + - diff --git a/packages/components/button/button.component.spec.ts b/packages/components/button/button.component.spec.ts index 2a50edd126..384cc68008 100644 --- a/packages/components/button/button.component.spec.ts +++ b/packages/components/button/button.component.spec.ts @@ -4,9 +4,12 @@ import { By } from '@angular/platform-browser'; import { NoopAnimationsModule } from '@angular/platform-browser/animations'; import { dispatchFakeEvent, + dispatchKeyboardEvent, + ENTER, KbqComponentColors, leftIconClassName, rightIconClassName, + SPACE, ThemePalette } from '@koobiq/components/core'; import { KbqDropdownModule } from '@koobiq/components/dropdown'; @@ -111,12 +114,45 @@ describe('KbqButton', () => { it('should not redirect if disabled', () => { const fixture = TestBed.createComponent(TestApp); const testComponent = fixture.debugElement.componentInstance; - const buttonDebugElement = fixture.debugElement.query(By.css('.kbq-button-overlay')); + const anchorElement: HTMLAnchorElement = fixture.debugElement.query(By.css('a')).nativeElement; testComponent.isDisabled = true; fixture.detectChanges(); - buttonDebugElement.nativeElement.click(); + const event = new MouseEvent('click', { bubbles: true, cancelable: true }); + + anchorElement.dispatchEvent(event); + + expect(testComponent.clickCount).toBe(0); + expect(event.defaultPrevented).toBe(true); + }); + + it('should halt ENTER and SPACE keydown if disabled', () => { + const fixture = TestBed.createComponent(TestApp); + const testComponent = fixture.debugElement.componentInstance; + const anchorElement: HTMLAnchorElement = fixture.debugElement.query(By.css('a')).nativeElement; + + testComponent.isDisabled = true; + fixture.detectChanges(); + + const enterEvent = dispatchKeyboardEvent(anchorElement, 'keydown', ENTER); + const spaceEvent = dispatchKeyboardEvent(anchorElement, 'keydown', SPACE); + + expect(enterEvent.defaultPrevented).toBe(true); + expect(spaceEvent.defaultPrevented).toBe(true); + }); + + it('should set aria-disabled if disabled', () => { + const fixture = TestBed.createComponent(TestApp); + const testComponent = fixture.debugElement.componentInstance; + const anchorElement: HTMLAnchorElement = fixture.debugElement.query(By.css('a')).nativeElement; + + expect(anchorElement.getAttribute('aria-disabled')).toBeNull(); + + testComponent.isDisabled = true; + fixture.detectChanges(); + + expect(anchorElement.getAttribute('aria-disabled')).toBe('true'); }); it('should remove tabindex if disabled', () => { @@ -171,6 +207,7 @@ describe('Button with icon', () => { KbqButtonTextIconLeftRightNgIfCaseTestApp, KbqButtonHtmlNodesNCountIconLeftRightNgIfCaseTestApp, KbqButtonTwoIconsCaseTestApp, + KbqButtonThreeIconsCaseTestApp, KbqButtonIconNgIfCaseTestApp ] }).compileComponents(); @@ -206,6 +243,23 @@ describe('Button with icon', () => { expect(fixture.debugElement.query(By.css(`.${buttonRightIconClassName}`))).not.toBeNull(); }); + it('should keep regular button styling and mark only outermost icons for more than 2 icons', () => { + const fixture = TestBed.createComponent(KbqButtonThreeIconsCaseTestApp); + + fixture.detectChanges(); + + const hostElement = fixture.debugElement.query(By.directive(KbqButtonCssStyler)).nativeElement; + + expect(hostElement.classList.contains('kbq-button')).toBeTruthy(); + expect(hostElement.classList.contains('kbq-button-icon')).toBeFalsy(); + + expect(fixture.debugElement.query(By.css(`#icon1.${leftIconClassName}`))).not.toBeNull(); + expect(fixture.debugElement.query(By.css(`#icon3.${rightIconClassName}`))).not.toBeNull(); + + expect(fixture.debugElement.query(By.css(`#icon2.${leftIconClassName}`))).toBeNull(); + expect(fixture.debugElement.query(By.css(`#icon2.${rightIconClassName}`))).toBeNull(); + }); + it('should add right css class when the previous sibling is an html element', () => { const fixture = TestBed.createComponent(KbqButtonHtmlIconRightCaseTestApp); @@ -581,6 +635,19 @@ class KbqButtonHtmlNodesNCountIconLeftRightNgIfCaseTestApp { }) class KbqButtonTwoIconsCaseTestApp {} +@Component({ + selector: 'kbq-button-three-icons-case-test-app', + imports: [KbqButtonModule, KbqIconModule], + template: ` + + ` +}) +class KbqButtonThreeIconsCaseTestApp {} + @Component({ selector: 'kbq-button-text-icon-case-test-app', imports: [KbqButtonModule, KbqIconModule], diff --git a/packages/components/button/button.component.ts b/packages/components/button/button.component.ts index 162a2930eb..90b48aeeae 100644 --- a/packages/components/button/button.component.ts +++ b/packages/components/button/button.component.ts @@ -14,23 +14,25 @@ import { forwardRef, inject, Input, + isDevMode, numberAttribute, OnDestroy, Renderer2, signal, - SkipSelf, + untracked, ViewChild, ViewEncapsulation } from '@angular/core'; -import { toObservable } from '@angular/core/rxjs-interop'; import { + ENTER, getNodesWithoutComments, KBQ_TITLE_TEXT_REF, KbqColorDirective, KbqComponentColors, KbqTitleTextRef, leftIconClassName, - rightIconClassName + rightIconClassName, + SPACE } from '@koobiq/components/core'; import { KbqIcon } from '@koobiq/components/icon'; @@ -43,6 +45,19 @@ export enum KbqButtonStyles { export const buttonLeftIconClassName = 'kbq-button-icon_left'; export const buttonRightIconClassName = 'kbq-button-icon_right'; +/** A button containing more icons than this keeps regular (non icon-button) styling. */ +const maxIconsForIconButton = 2; + +/** + * Applies the `kbq-button`/`kbq-button-icon` host class and the left/right icon modifier classes. + * + * A button is treated as an icon button when its projected content consists only of `KbqIcon`s + * and there are at most 2 of them. When icons are mixed with other content, only the outermost + * icons receive the left/right classes. + * + * Must be used together with `KbqButton` (both match `[kbq-button]`): icon detection relies on + * the `.kbq-button-wrapper` element rendered by the component's template. + */ @Directive({ selector: '[kbq-button]', host: { @@ -55,14 +70,31 @@ export class KbqButtonCssStyler implements AfterContentInit { nativeElement: HTMLElement; - isIconButton: boolean = false; + /** Whether the button contains only icons (at most 2). */ + get isIconButton(): boolean { + return this._isIconButton(); + } + + private readonly _isIconButton = signal(false); + + private leftIcon: HTMLElement | null = null; + private rightIcon: HTMLElement | null = null; constructor( elementRef: ElementRef, - private renderer: Renderer2, - @SkipSelf() private cdr: ChangeDetectorRef + private renderer: Renderer2 ) { this.nativeElement = elementRef.nativeElement; + + // The contentChildren query tracks only KbqIcon instances, while icon placement also + // depends on sibling text nodes that are invisible to the query — those are covered by + // the MutationObserver in the component template. This effect covers icon creation and + // removal (e.g. via @if) while the observer is disabled for icon-less buttons. + effect(() => { + this.icons(); + + untracked(() => this.updateClassModifierForIcons()); + }); } ngAfterContentInit() { @@ -70,47 +102,62 @@ export class KbqButtonCssStyler implements AfterContentInit { } updateClassModifierForIcons() { - this.renderer.removeClass(this.nativeElement, buttonLeftIconClassName); - this.renderer.removeClass(this.nativeElement, buttonRightIconClassName); - this.icons() - .map((item) => item.getHostElement()) - .forEach((iconHostElement) => { - this.renderer.removeClass(iconHostElement, leftIconClassName); - this.renderer.removeClass(iconHostElement, rightIconClassName); - }); - - const twoIcons = 2; - const filteredNodesWithoutComments = getNodesWithoutComments( - this.nativeElement.querySelector('.kbq-button-wrapper')!.childNodes as NodeList - ); + const wrapper = this.nativeElement.querySelector('.kbq-button-wrapper'); + + if (!wrapper) { + if (isDevMode()) { + // eslint-disable-next-line no-console + console.warn('KbqButtonCssStyler should be imported together with KbqButton.'); + } + + return; + } const icons = this.icons(); - const currentIsIconButtonValue = - !!icons.length && icons.length === filteredNodesWithoutComments.length && icons.length <= twoIcons; + const nodes = getNodesWithoutComments(wrapper.childNodes); + + this._isIconButton.set( + !!icons.length && icons.length === nodes.length && icons.length <= maxIconsForIconButton + ); + + let leftIcon: HTMLElement | null = null; + let rightIcon: HTMLElement | null = null; + + if (icons.length && nodes.length > 1) { + for (const icon of icons) { + const iconHostElement = icon.getHostElement(); + const iconIndex = nodes.indexOf(iconHostElement); - if (currentIsIconButtonValue !== this.isIconButton) { - this.isIconButton = currentIsIconButtonValue; - this.cdr.detectChanges(); + if (iconIndex === 0) leftIcon = iconHostElement; + + if (iconIndex === nodes.length - 1) rightIcon = iconHostElement; + } } - const iconsValue = this.icons(); + this.updateIconClass(this.leftIcon, leftIcon, leftIconClassName, buttonLeftIconClassName); + this.updateIconClass(this.rightIcon, rightIcon, rightIconClassName, buttonRightIconClassName); - if (iconsValue.length && filteredNodesWithoutComments.length > 1) { - iconsValue - .map((item) => item.getHostElement()) - .forEach((iconHostElement) => { - const iconIndex = filteredNodesWithoutComments.findIndex((node) => node === iconHostElement); + this.leftIcon = leftIcon; + this.rightIcon = rightIcon; + } - if (iconIndex === 0) { - this.renderer.addClass(iconHostElement, leftIconClassName); - this.renderer.addClass(this.nativeElement, buttonLeftIconClassName); - } + private updateIconClass( + previous: HTMLElement | null, + current: HTMLElement | null, + iconClassName: string, + buttonClassName: string + ) { + if (previous === current) return; - if (iconIndex === filteredNodesWithoutComments.length - 1) { - this.renderer.addClass(iconHostElement, rightIconClassName); - this.renderer.addClass(this.nativeElement, buttonRightIconClassName); - } - }); + if (previous) { + this.renderer.removeClass(previous, iconClassName); + } + + if (current) { + this.renderer.addClass(current, iconClassName); + this.renderer.addClass(this.nativeElement, buttonClassName); + } else { + this.renderer.removeClass(this.nativeElement, buttonClassName); } } } @@ -129,10 +176,11 @@ export class KbqButtonCssStyler implements AfterContentInit { encapsulation: ViewEncapsulation.None, host: { '[attr.disabled]': 'disabled || null', + '[attr.aria-disabled]': 'disabled || null', '[class.kbq-disabled]': 'disabled', '[attr.tabIndex]': 'tabIndex', '[class]': 'kbqStyle', - '(focus)': 'onFocus($event)', + '(focus)': 'onFocus()', '(blur)': 'onBlur()' } }) @@ -141,7 +189,7 @@ export class KbqButton extends KbqColorDirective implements OnDestroy, AfterView hasFocus: boolean = false; - @ViewChild('kbqTitleText', { static: false }) textElement: ElementRef; + @ViewChild('kbqTitleText') textElement: ElementRef; // TODO: Skipped for migration because: // Accessor inputs cannot be migrated as they are too complex. @@ -164,16 +212,13 @@ export class KbqButton extends KbqColorDirective implements OnDestroy, AfterView // Accessor inputs cannot be migrated as they are too complex. @Input({ transform: booleanAttribute }) get disabled(): boolean { - return this._disabled; + return this.disabledSignal(); } set disabled(value: boolean) { this.disabledSignal.set(value); } - // @todo 20 In the next major release this line will be deleted. - private _disabled: boolean; - /** @docs-private */ readonly disabledSignal = signal(false); @@ -192,17 +237,20 @@ export class KbqButton extends KbqColorDirective implements OnDestroy, AfterView constructor( private focusMonitor: FocusMonitor, - private styler: KbqButtonCssStyler + protected styler: KbqButtonCssStyler ) { super(); this.color = KbqComponentColors.ContrastFade; this.setDefaultColor(KbqComponentColors.ContrastFade); - // @todo 20 In the next major release this line will be deleted. - toObservable(this.disabledSignal).subscribe((value) => (this._disabled = value)); - - effect(() => (this.disabledSignal() ? this.stopFocusMonitor() : this.runFocusMonitor())); + // Native capture-phase listeners instead of host listeners: Angular coalesces listeners + // for the same event on the same element, so stopImmediatePropagation from a host listener + // would not stop consumer-bound handlers. Matters for hosts only — + // a disabled native +   + +   + +   + + + + +
+ +
+

Text ellipsis (width-constrained)

+ + + +   + + +   + + +
+ +
+
+ ` +}) +class KbqButtonLeftIconSlotReorderTestApp {} + +@Component({ + selector: 'kbq-button-right-icon-slot-reorder-test-app', + imports: [KbqButtonModule, KbqIconModule], + template: ` + + ` +}) +class KbqButtonRightIconSlotReorderTestApp {} + +@Component({ + selector: 'kbq-button-left-right-icon-slot-test-app', + imports: [KbqButtonModule, KbqIconModule], + template: ` + + ` +}) +class KbqButtonLeftRightIconSlotTestApp {} + +@Component({ + selector: 'kbq-button-two-icons-slot-test-app', + imports: [KbqButtonModule, KbqIconModule], + template: ` + + ` +}) +class KbqButtonTwoIconsSlotTestApp {} diff --git a/packages/components/button/button.component.ts b/packages/components/button/button.component.ts index 90b48aeeae..a2d2ab2188 100644 --- a/packages/components/button/button.component.ts +++ b/packages/components/button/button.component.ts @@ -48,6 +48,9 @@ export const buttonRightIconClassName = 'kbq-button-icon_right'; /** A button containing more icons than this keeps regular (non icon-button) styling. */ const maxIconsForIconButton = 2; +/** `Node.COMMENT_NODE` nodeType value. */ +const COMMENT_NODE = 8; + /** * Applies the `kbq-button`/`kbq-button-icon` host class and the left/right icon modifier classes. * @@ -114,23 +117,41 @@ export class KbqButtonCssStyler implements AfterContentInit { } const icons = this.icons(); - const nodes = getNodesWithoutComments(wrapper.childNodes); + const textElement = wrapper.querySelector('.kbq-button-text'); + + // Build an ordered list of "effective" content nodes: the left-slot content, then the + // default-slot content flattened out of `.kbq-button-text`, then the right-slot content. + // Flattening the text span keeps legacy ` Text` markup (projected into the + // default slot) working: those icons live inside `.kbq-button-text`, but for placement + // they must be treated as direct siblings of the text, exactly as before the text span + // existed. With no marker slots this list equals the old wrapper children. + const effectiveNodes: Node[] = []; + + for (const node of Array.from(wrapper.childNodes)) { + if (node.nodeType === COMMENT_NODE) continue; + + if (node === textElement) { + effectiveNodes.push(...getNodesWithoutComments((node as HTMLElement).childNodes)); + } else { + effectiveNodes.push(node); + } + } this._isIconButton.set( - !!icons.length && icons.length === nodes.length && icons.length <= maxIconsForIconButton + !!icons.length && icons.length === effectiveNodes.length && icons.length <= maxIconsForIconButton ); let leftIcon: HTMLElement | null = null; let rightIcon: HTMLElement | null = null; - if (icons.length && nodes.length > 1) { + if (icons.length && effectiveNodes.length > 1) { for (const icon of icons) { const iconHostElement = icon.getHostElement(); - const iconIndex = nodes.indexOf(iconHostElement); + const iconIndex = effectiveNodes.indexOf(iconHostElement); if (iconIndex === 0) leftIcon = iconHostElement; - if (iconIndex === nodes.length - 1) rightIcon = iconHostElement; + if (iconIndex === effectiveNodes.length - 1) rightIcon = iconHostElement; } } @@ -191,6 +212,9 @@ export class KbqButton extends KbqColorDirective implements OnDestroy, AfterView @ViewChild('kbqTitleText') textElement: ElementRef; + /** The flex row that lays out the icons and text, used as the overflow width constraint. */ + @ViewChild('parentTextElement') parentTextElement: ElementRef; + // TODO: Skipped for migration because: // Accessor inputs cannot be migrated as they are too complex. @Input() diff --git a/packages/components/button/button.module.ts b/packages/components/button/button.module.ts index 45a53bdb80..fe03c6953f 100644 --- a/packages/components/button/button.module.ts +++ b/packages/components/button/button.module.ts @@ -3,6 +3,7 @@ import { ObserversModule } from '@angular/cdk/observers'; import { PlatformModule } from '@angular/cdk/platform'; import { NgModule } from '@angular/core'; import { KbqButtonGroup, KbqButtonGroupRoot } from './button-group'; +import { KbqButtonLeftIcon, KbqButtonRightIcon } from './button-icon'; import { KbqButton, KbqButtonCssStyler } from './button.component'; import { KbqButtonDropdownTrigger } from './button.dropdown-trigger.directive'; @@ -14,12 +15,16 @@ import { KbqButtonDropdownTrigger } from './button.dropdown-trigger.directive'; KbqButtonDropdownTrigger, KbqButton, KbqButtonCssStyler, + KbqButtonLeftIcon, + KbqButtonRightIcon, KbqButtonGroup, KbqButtonGroupRoot ], exports: [ KbqButton, KbqButtonCssStyler, + KbqButtonLeftIcon, + KbqButtonRightIcon, KbqButtonDropdownTrigger, KbqButtonGroup, KbqButtonGroupRoot diff --git a/packages/components/button/public-api.ts b/packages/components/button/public-api.ts index 16d7ff2f87..c49f8abb1d 100644 --- a/packages/components/button/public-api.ts +++ b/packages/components/button/public-api.ts @@ -1,4 +1,5 @@ export * from './button-group'; +export * from './button-icon'; export * from './button.component'; export * from './button.dropdown-trigger.directive'; export * from './button.module'; diff --git a/packages/docs-examples/components/button/button-content/button-content-example.ts b/packages/docs-examples/components/button/button-content/button-content-example.ts index 78234a914e..ef03459fb3 100644 --- a/packages/docs-examples/components/button/button-content/button-content-example.ts +++ b/packages/docs-examples/components/button/button-content/button-content-example.ts @@ -40,6 +40,14 @@ import { KbqIconModule } from '@koobiq/components/icon';
+
+
Slots (order-independent)
+ +
`, styleUrls: ['button-content-example.css'], diff --git a/tools/public_api_guard/components/button.api.md b/tools/public_api_guard/components/button.api.md index 9402b88f19..b34e8219b8 100644 --- a/tools/public_api_guard/components/button.api.md +++ b/tools/public_api_guard/components/button.api.md @@ -58,6 +58,7 @@ export class KbqButton extends KbqColorDirective implements OnDestroy, AfterView onBlur(): void; // (undocumented) onFocus(): void; + parentTextElement: ElementRef; // (undocumented) projectContentChanged(): void; // (undocumented) @@ -68,7 +69,7 @@ export class KbqButton extends KbqColorDirective implements OnDestroy, AfterView // (undocumented) textElement: ElementRef; // (undocumented) - static ɵcmp: i0.ɵɵComponentDeclaration; + static ɵcmp: i0.ɵɵComponentDeclaration; // (undocumented) static ɵfac: i0.ɵɵFactoryDeclaration; } @@ -128,6 +129,14 @@ export class KbqButtonGroupRoot extends KbqColorDirective { static ɵfac: i0.ɵɵFactoryDeclaration; } +// @public +export class KbqButtonLeftIcon { + // (undocumented) + static ɵdir: i0.ɵɵDirectiveDeclaration; + // (undocumented) + static ɵfac: i0.ɵɵFactoryDeclaration; +} + // @public (undocumented) export class KbqButtonModule { // (undocumented) @@ -135,7 +144,15 @@ export class KbqButtonModule { // (undocumented) static ɵinj: i0.ɵɵInjectorDeclaration; // (undocumented) - static ɵmod: i0.ɵɵNgModuleDeclaration; + static ɵmod: i0.ɵɵNgModuleDeclaration; +} + +// @public +export class KbqButtonRightIcon { + // (undocumented) + static ɵdir: i0.ɵɵDirectiveDeclaration; + // (undocumented) + static ɵfac: i0.ɵɵFactoryDeclaration; } // @public (undocumented) From ffa27541598c53fc29300603399b59522bab9b8f Mon Sep 17 00:00:00 2001 From: lskramarov Date: Thu, 11 Jun 2026 14:21:37 +0300 Subject: [PATCH 3/6] fix(button): halt arrow keydown on disabled dropdown-trigger hosts (#DS-4364) - extend the disabled keydown guard to DOWN/LEFT/RIGHT arrows, not just ENTER/SPACE, since KbqDropdownTrigger also opens on those (Tab/Escape left untouched so focus can still leave a disabled but focusable host); fix the misleading comment - dedupe the COMMENT_NODE literal by reusing getNodesWithoutComments at both levels - tests: assert a disabled dropdown trigger stays closed on click/ENTER/SPACE/arrow (guards stopImmediatePropagation), the effect()-driven reveal while the content observer is disabled, aria-disabled removal on re-enable, parentTextElement wrapping textElement, and the dev-mode warning when the styler is used without KbqButton --- .../button/button.component.spec.ts | 105 +++++++++++++++++- .../components/button/button.component.ts | 18 +-- 2 files changed, 114 insertions(+), 9 deletions(-) diff --git a/packages/components/button/button.component.spec.ts b/packages/components/button/button.component.spec.ts index 07dd40b6ea..80cd9480b9 100644 --- a/packages/components/button/button.component.spec.ts +++ b/packages/components/button/button.component.spec.ts @@ -5,6 +5,7 @@ import { NoopAnimationsModule } from '@angular/platform-browser/animations'; import { dispatchFakeEvent, dispatchKeyboardEvent, + DOWN_ARROW, ENTER, KbqComponentColors, leftIconClassName, @@ -12,7 +13,7 @@ import { SPACE, ThemePalette } from '@koobiq/components/core'; -import { KbqDropdownModule } from '@koobiq/components/dropdown'; +import { KbqDropdownModule, KbqDropdownTrigger } from '@koobiq/components/dropdown'; import { KbqIconModule } from '@koobiq/components/icon'; import { buttonLeftIconClassName, @@ -154,6 +155,12 @@ describe('KbqButton', () => { fixture.detectChanges(); expect(anchorElement.getAttribute('aria-disabled')).toBe('true'); + + // `disabled || null` must remove the attribute (not set "false") when re-enabled + testComponent.isDisabled = false; + fixture.detectChanges(); + + expect(anchorElement.getAttribute('aria-disabled')).toBeNull(); }); it('should remove tabindex if disabled', () => { @@ -182,6 +189,32 @@ describe('KbqButton', () => { expect(trigger.nativeElement.classList.contains('kbq-active')).toBeTruthy(); }); + it('should not open an associated dropdown when the disabled button is clicked or keyboard-activated', () => { + const fixture: ComponentFixture = + TestBed.createComponent(DisabledButtonDropdownTrigger); + + fixture.detectChanges(); + + const { dropdownTrigger, trigger } = fixture.componentInstance; + const anchorElement = trigger().nativeElement as HTMLAnchorElement; + + // click is halted by the capture-phase listener (stopImmediatePropagation), so the + // dropdown trigger's own click handler never runs and the dropdown stays closed + dispatchFakeEvent(anchorElement, 'click'); + fixture.detectChanges(); + + expect(dropdownTrigger().opened).toBe(false); + expect(anchorElement.classList.contains('kbq-active')).toBe(false); + + // ENTER / SPACE and the arrow keys the trigger opens on are all halted too + [ENTER, SPACE, DOWN_ARROW].forEach((keyCode) => { + dispatchKeyboardEvent(anchorElement, 'keydown', keyCode); + fixture.detectChanges(); + + expect(dropdownTrigger().opened).toBe(false); + }); + }); + it('should handle a click on the button', () => { const fixture = TestBed.createComponent(TestApp); const testComponent = fixture.debugElement.componentInstance; @@ -362,6 +395,28 @@ describe('Button with icon', () => { done(); }); }); + + it('should switch to kbq-button-icon via the effect when an icon is revealed while the content observer is disabled', (done) => { + const fixture = TestBed.createComponent(KbqButtonIconNgIfCaseTestApp); + const debugElement = fixture.debugElement.query(By.directive(KbqButtonCssStyler)); + + // start with no icon: cdkObserveContent is disabled (styler.icons() is empty), so the + // styler's effect() — not the MutationObserver — is the only thing that can recompute + fixture.componentInstance.visible = false; + fixture.detectChanges(); + + expect(debugElement.nativeElement.classList.contains('kbq-button-icon')).toBeFalsy(); + expect(debugElement.nativeElement.classList.contains('kbq-button')).toBeTruthy(); + + fixture.componentInstance.visible = true; + fixture.detectChanges(); + + setTimeout(() => { + expect(debugElement.nativeElement.classList.contains('kbq-button-icon')).toBeTruthy(); + expect(debugElement.nativeElement.classList.contains('kbq-button')).toBeFalsy(); + done(); + }); + }); }); describe('Button with explicit icon slots', () => { @@ -504,6 +559,30 @@ describe('Button text container', () => { expect(button.textElement.nativeElement.classList.contains('kbq-button-text')).toBe(true); expect(button.parentTextElement.nativeElement.classList.contains('kbq-button-wrapper')).toBe(true); + + // parent (width container) must wrap the child (measured text) — guards against the two + // ViewChildren being swapped, which would break kbq-title overflow detection + expect(button.parentTextElement.nativeElement.contains(button.textElement.nativeElement)).toBe(true); + }); +}); + +describe('KbqButtonCssStyler without KbqButton', () => { + beforeEach(() => { + TestBed.configureTestingModule({ + imports: [StylerOnlyTestApp] + }).compileComponents(); + }); + + it('should warn in dev mode when there is no .kbq-button-wrapper (styler used without KbqButton)', () => { + const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + + const fixture = TestBed.createComponent(StylerOnlyTestApp); + + fixture.detectChanges(); + + expect(warnSpy).toHaveBeenCalledWith('KbqButtonCssStyler should be imported together with KbqButton.'); + + warnSpy.mockRestore(); }); }); @@ -826,6 +905,20 @@ class ButtonDropdownTrigger { backdropClass: string; } +@Component({ + imports: [KbqButtonModule, KbqDropdownModule], + template: ` + Toggle dropdown + + + + ` +}) +class DisabledButtonDropdownTrigger { + readonly trigger = viewChild.required('triggerEl', { read: ElementRef }); + readonly dropdownTrigger = viewChild.required(KbqDropdownTrigger); +} + @Component({ imports: [KbqButtonModule], template: ` @@ -911,3 +1004,13 @@ class KbqButtonLeftRightIconSlotTestApp {} ` }) class KbqButtonTwoIconsSlotTestApp {} + +@Component({ + selector: 'styler-only-test-app', + // KbqButton is intentionally NOT imported, so the host has no .kbq-button-wrapper + imports: [KbqButtonCssStyler], + template: ` +
+ ` +}) +class StylerOnlyTestApp {} diff --git a/packages/components/button/button.component.ts b/packages/components/button/button.component.ts index a2d2ab2188..b7a44fbd9a 100644 --- a/packages/components/button/button.component.ts +++ b/packages/components/button/button.component.ts @@ -24,13 +24,16 @@ import { ViewEncapsulation } from '@angular/core'; import { + DOWN_ARROW, ENTER, getNodesWithoutComments, KBQ_TITLE_TEXT_REF, KbqColorDirective, KbqComponentColors, KbqTitleTextRef, + LEFT_ARROW, leftIconClassName, + RIGHT_ARROW, rightIconClassName, SPACE } from '@koobiq/components/core'; @@ -48,9 +51,6 @@ export const buttonRightIconClassName = 'kbq-button-icon_right'; /** A button containing more icons than this keeps regular (non icon-button) styling. */ const maxIconsForIconButton = 2; -/** `Node.COMMENT_NODE` nodeType value. */ -const COMMENT_NODE = 8; - /** * Applies the `kbq-button`/`kbq-button-icon` host class and the left/right icon modifier classes. * @@ -127,9 +127,7 @@ export class KbqButtonCssStyler implements AfterContentInit { // existed. With no marker slots this list equals the old wrapper children. const effectiveNodes: Node[] = []; - for (const node of Array.from(wrapper.childNodes)) { - if (node.nodeType === COMMENT_NODE) continue; - + for (const node of getNodesWithoutComments(wrapper.childNodes)) { if (node === textElement) { effectiveNodes.push(...getNodesWithoutComments((node as HTMLElement).childNodes)); } else { @@ -272,7 +270,8 @@ export class KbqButton extends KbqColorDirective implements OnDestroy, AfterView // for the same event on the same element, so stopImmediatePropagation from a host listener // would not stop consumer-bound handlers. Matters for hosts only — // a disabled native