-
Notifications
You must be signed in to change notification settings - Fork 1.8k
[CL-1115] accordion component #20508
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 5 commits
2de2ca1
bb707b6
2ed6977
e87bf9b
f4dba2c
9e3112d
d57d173
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| import { booleanAttribute, ChangeDetectionStrategy, Component, input, signal } from "@angular/core"; | ||
|
|
||
| import { AccordionVariant } from "./accordion.component"; | ||
|
|
||
| @Component({ | ||
| selector: "bit-accordion-group", | ||
| template: "<ng-content />", | ||
| changeDetection: ChangeDetectionStrategy.OnPush, | ||
| }) | ||
| export class AccordionGroupComponent { | ||
| readonly singleSelect = input(false, { transform: booleanAttribute }); | ||
| readonly variant = input<AccordionVariant>("default"); | ||
|
|
||
| readonly _activeAccordionId = signal<string | null>(null); | ||
|
|
||
| notifyOpened(id: string) { | ||
| if (this.singleSelect()) { | ||
| this._activeAccordionId.set(id); | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| <button | ||
| type="button" | ||
| data-accordion-trigger | ||
| [id]="triggerId" | ||
| [class]="triggerClassList()" | ||
| [disabled]="disabled() || null" | ||
| [attr.aria-expanded]="open()" | ||
| [attr.aria-controls]="contentId" | ||
| (click)="toggle()" | ||
| > | ||
| @if (startIcon()) { | ||
| <bit-icon-tile [icon]="startIcon()" [size]="iconTileSize()" variant="contrast"></bit-icon-tile> | ||
| } | ||
| <div class="tw-flex tw-flex-col tw-flex-1 tw-min-w-0 tw-gap-1 tw-items-start"> | ||
| <span [class]="headingClassList()">{{ heading() }}</span> | ||
| @if (size() === "default" && subtitle()) { | ||
| <span [class]="subtitleClassList()">{{ subtitle() }}</span> | ||
| } | ||
| </div> | ||
| <ng-content select="[end]"></ng-content> | ||
|
BryanCunningham marked this conversation as resolved.
Outdated
|
||
| <bit-icon | ||
| [name]="open() ? 'bwi-angle-up' : 'bwi-angle-down'" | ||
| [class]="chevronClasses()" | ||
| ></bit-icon> | ||
| </button> | ||
| @if (open()) { | ||
| <div | ||
| data-accordion-content | ||
| [id]="contentId" | ||
| role="region" | ||
| [attr.aria-labelledby]="triggerId" | ||
| [class]="contentClassList()" | ||
| > | ||
| <ng-content></ng-content> | ||
| </div> | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,156 @@ | ||
| import { ComponentFixture, TestBed } from "@angular/core/testing"; | ||
|
|
||
| import { AccordionComponent } from "./accordion.component"; | ||
|
|
||
| describe("AccordionComponent", () => { | ||
| let component: AccordionComponent; | ||
| let fixture: ComponentFixture<AccordionComponent>; | ||
|
|
||
| beforeEach(async () => { | ||
| await TestBed.configureTestingModule({ | ||
| imports: [AccordionComponent], | ||
| }).compileComponents(); | ||
|
|
||
| fixture = TestBed.createComponent(AccordionComponent); | ||
| component = fixture.componentInstance; | ||
| fixture.componentRef.setInput("heading", "Test Heading"); | ||
| fixture.detectChanges(); | ||
| }); | ||
|
|
||
| it("creates", () => { | ||
| expect(component).toBeTruthy(); | ||
| }); | ||
|
|
||
| describe("default state", () => { | ||
| it("is collapsed by default", () => { | ||
| expect(component.open()).toBe(false); | ||
| }); | ||
|
|
||
| it("does not render content panel when collapsed", () => { | ||
| expect(fixture.nativeElement.querySelector(`#${component.contentId}`)).toBeNull(); | ||
| }); | ||
|
|
||
| it("button has aria-expanded=false when closed", () => { | ||
| expect(fixture.nativeElement.querySelector("button").getAttribute("aria-expanded")).toBe( | ||
| "false", | ||
| ); | ||
| }); | ||
|
|
||
| it("shows chevron-down icon when collapsed", () => { | ||
| expect(fixture.nativeElement.querySelector("bit-icon").classList).toContain("bwi-angle-down"); | ||
| }); | ||
| }); | ||
|
|
||
| describe("toggle", () => { | ||
| it("opens when button is clicked", () => { | ||
| fixture.nativeElement.querySelector("button").click(); | ||
| fixture.detectChanges(); | ||
| expect(component.open()).toBe(true); | ||
| }); | ||
|
|
||
| it("closes again on second click", () => { | ||
| fixture.componentRef.setInput("open", true); | ||
| fixture.detectChanges(); | ||
| fixture.nativeElement.querySelector("button").click(); | ||
| fixture.detectChanges(); | ||
| expect(component.open()).toBe(false); | ||
| }); | ||
|
|
||
| it("renders content panel when open", () => { | ||
| fixture.componentRef.setInput("open", true); | ||
| fixture.detectChanges(); | ||
| expect(fixture.nativeElement.querySelector(`#${component.contentId}`)).toBeTruthy(); | ||
| }); | ||
|
|
||
| it("button has aria-expanded=true when open", () => { | ||
| fixture.componentRef.setInput("open", true); | ||
| fixture.detectChanges(); | ||
| expect(fixture.nativeElement.querySelector("button").getAttribute("aria-expanded")).toBe( | ||
| "true", | ||
| ); | ||
| }); | ||
|
|
||
| it("shows chevron-up icon when open", () => { | ||
| fixture.componentRef.setInput("open", true); | ||
| fixture.detectChanges(); | ||
| expect(fixture.nativeElement.querySelector("bit-icon").classList).toContain("bwi-angle-up"); | ||
| }); | ||
| }); | ||
|
|
||
| describe("accessibility", () => { | ||
| it("button aria-controls matches content panel id when open", () => { | ||
| fixture.componentRef.setInput("open", true); | ||
| fixture.detectChanges(); | ||
| const btn = fixture.nativeElement.querySelector("button"); | ||
| const panel = fixture.nativeElement.querySelector(`#${component.contentId}`); | ||
| expect(btn.getAttribute("aria-controls")).toBe(panel.id); | ||
| }); | ||
|
|
||
| it("content panel has role=region when open", () => { | ||
| fixture.componentRef.setInput("open", true); | ||
| fixture.detectChanges(); | ||
| expect( | ||
| fixture.nativeElement.querySelector(`#${component.contentId}`).getAttribute("role"), | ||
| ).toBe("region"); | ||
| }); | ||
|
|
||
| it("content panel aria-labelledby points to trigger button id when open", () => { | ||
| fixture.componentRef.setInput("open", true); | ||
| fixture.detectChanges(); | ||
| expect( | ||
| fixture.nativeElement | ||
| .querySelector(`#${component.contentId}`) | ||
| .getAttribute("aria-labelledby"), | ||
| ).toBe(component.triggerId); | ||
| }); | ||
|
|
||
| it("chevron icon has aria-hidden=true", () => { | ||
| expect(fixture.nativeElement.querySelector("bit-icon").getAttribute("aria-hidden")).toBe( | ||
| "true", | ||
| ); | ||
| }); | ||
| }); | ||
|
|
||
| describe("disabled", () => { | ||
| beforeEach(() => { | ||
| fixture.componentRef.setInput("disabled", true); | ||
| fixture.detectChanges(); | ||
| }); | ||
|
|
||
| it("does not toggle when clicked", () => { | ||
| fixture.nativeElement.querySelector("button").click(); | ||
| fixture.detectChanges(); | ||
| expect(component.open()).toBe(false); | ||
| }); | ||
|
|
||
| it("button has disabled attribute", () => { | ||
| expect(fixture.nativeElement.querySelector("button").hasAttribute("disabled")).toBe(true); | ||
| }); | ||
| }); | ||
|
|
||
| describe("subtitle", () => { | ||
| it("shows subtitle when provided", () => { | ||
| fixture.componentRef.setInput("subtitle", "My subtitle"); | ||
| fixture.detectChanges(); | ||
| const spans = fixture.nativeElement.querySelectorAll("button span"); | ||
| const found = Array.from(spans).some((el: any) => el.textContent.trim() === "My subtitle"); | ||
| expect(found).toBe(true); | ||
| }); | ||
|
|
||
| it("does not render subtitle span when not provided", () => { | ||
| fixture.detectChanges(); | ||
| const spans = fixture.nativeElement.querySelectorAll("button span"); | ||
| expect(spans.length).toBe(1); | ||
| }); | ||
| }); | ||
|
|
||
| describe("host classes", () => { | ||
|
BryanCunningham marked this conversation as resolved.
Outdated
|
||
| it("has tw-rounded-xl", () => { | ||
| expect(fixture.nativeElement.classList).toContain("tw-rounded-xl"); | ||
| }); | ||
|
|
||
| it("has tw-border", () => { | ||
| expect(fixture.nativeElement.classList).toContain("tw-border"); | ||
| }); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,155 @@ | ||
| import { | ||
| booleanAttribute, | ||
| ChangeDetectionStrategy, | ||
| Component, | ||
| computed, | ||
| effect, | ||
| inject, | ||
| input, | ||
| model, | ||
| untracked, | ||
| } from "@angular/core"; | ||
|
|
||
| import { IconComponent } from "../icon"; | ||
| import { IconTileComponent } from "../icon-tile"; | ||
| import { BitwardenIcon } from "../shared/icon"; | ||
|
|
||
| import { AccordionGroupComponent } from "./accordion-group.component"; | ||
|
|
||
| export type AccordionSize = "sm" | "default"; | ||
|
|
||
| export type AccordionVariant = "default" | "subtle"; | ||
|
|
||
| let nextId = 0; | ||
|
|
||
| @Component({ | ||
| selector: "bit-accordion", | ||
| templateUrl: "./accordion.component.html", | ||
| changeDetection: ChangeDetectionStrategy.OnPush, | ||
| imports: [IconComponent, IconTileComponent], | ||
| host: { | ||
| "[class]": "hostClassList()", | ||
| }, | ||
| }) | ||
| export class AccordionComponent { | ||
| private readonly group = inject(AccordionGroupComponent, { optional: true }); | ||
|
|
||
| constructor() { | ||
| effect(() => { | ||
| const activeId = this.group?._activeAccordionId(); | ||
|
BryanCunningham marked this conversation as resolved.
Outdated
|
||
| if (activeId != null && activeId !== this._baseId) { | ||
| untracked(() => this.open.set(false)); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are closing this accordion imperatively if the parent group component is single select and is controlling which accordion is open. We had to use |
||
| } | ||
| }); | ||
| } | ||
|
|
||
| readonly heading = input.required<string>(); | ||
|
BryanCunningham marked this conversation as resolved.
Outdated
|
||
| readonly subtitle = input<string>(); | ||
| readonly open = model<boolean>(false); | ||
| readonly startIcon = input<BitwardenIcon>(); | ||
| readonly disabled = input(false, { transform: booleanAttribute }); | ||
| readonly size = input<AccordionSize>("default"); | ||
| readonly variant = input<AccordionVariant>("default"); | ||
|
|
||
| protected readonly resolvedVariant = computed(() => this.group?.variant() ?? this.variant()); | ||
|
|
||
| protected readonly _baseId = `bit-accordion-${nextId++}`; | ||
| readonly triggerId = `${this._baseId}-trigger`; | ||
| readonly contentId = `${this._baseId}-content`; | ||
|
|
||
| protected toggle() { | ||
| if (!this.disabled()) { | ||
| this.open.update((o) => !o); | ||
| if (this.open()) { | ||
| this.group?.notifyOpened(this._baseId); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| protected readonly hostClassList = computed(() => | ||
| [ | ||
| "tw-block", | ||
| "tw-border", | ||
| "tw-border-solid", | ||
| "tw-border-border-base", | ||
| "tw-rounded-xl", | ||
| ...(this.group | ||
| ? [ | ||
| // Collapse inner radii and borders when stacked inside a group | ||
| "[&:not(:first-of-type)]:tw-rounded-t-none", | ||
| "[&:not(:last-of-type)]:tw-rounded-b-none", | ||
| "[&:not(:last-of-type)]:tw-border-b-0", | ||
| // Mirror those overrides onto the child button and content panel | ||
| "[&:not(:first-of-type)>[data-accordion-trigger]]:tw-rounded-t-none", | ||
| "[&:not(:last-of-type)>[data-accordion-trigger]]:tw-rounded-b-none", | ||
| "[&:not(:last-of-type)>[data-accordion-content]]:tw-rounded-b-none", | ||
| ] | ||
| : []), | ||
| ].join(" "), | ||
| ); | ||
|
|
||
| protected readonly triggerClassList = computed(() => | ||
| [ | ||
| "tw-flex", | ||
| "tw-items-center", | ||
| "tw-gap-3", | ||
| "tw-w-full", | ||
| "tw-border-0", | ||
| "tw-text-start", | ||
| "tw-cursor-pointer", | ||
| "tw-transition-colors", | ||
| "tw-rounded-t-xl", | ||
| this.open() ? "" : "tw-rounded-b-xl", | ||
| this.resolvedVariant() === "default" ? "tw-bg-bg-secondary" : "tw-bg-bg-primary", | ||
| "enabled:hover:tw-bg-bg-hover", | ||
| "focus-visible:tw-outline-none", | ||
| "focus-visible:tw-ring-2", | ||
| "focus-visible:tw-ring-inset", | ||
| "focus-visible:tw-ring-border-focus", | ||
| "focus-visible:tw-border-border-focus", | ||
| "disabled:tw-cursor-not-allowed", | ||
| "disabled:tw-text-fg-inactive", | ||
| ...this.triggerSizeClasses(), | ||
| ].join(" "), | ||
| ); | ||
|
|
||
| private readonly triggerSizeClasses = computed((): string[] => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are other styles inline as ternaries; does this one need to be separate?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not necessarily. I can move it. Generally, I like the idea of all the styles being in the component vs the template. I guess I should just commit one way or the other :)
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For this one I think it is still used in the component side not the template. It's just split out into another function by itself
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated to be inline
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I'm not sure why I did it that way. |
||
| if (this.size() === "sm") { | ||
| return ["tw-p-3"]; | ||
| } | ||
| return ["tw-p-4"]; | ||
| }); | ||
|
|
||
| protected readonly iconTileSize = computed(() => (this.size() === "sm" ? "base" : "lg")); | ||
|
|
||
| protected readonly headingClassList = computed(() => | ||
| [ | ||
| "tw-font-medium", | ||
| "tw-leading-6", | ||
| this.size() === "sm" ? "tw-text-base" : "tw-text-lg", | ||
| this.disabled() ? "tw-text-fg-inactive" : "tw-text-fg-heading", | ||
| ].join(" "), | ||
| ); | ||
|
|
||
| protected readonly subtitleClassList = computed(() => | ||
| ["tw-text-sm/5", this.disabled() ? "tw-text-fg-inactive" : "tw-text-fg-body"].join(" "), | ||
| ); | ||
|
|
||
| protected readonly chevronClasses = computed(() => | ||
| [ | ||
| "tw-text-xl", | ||
| "tw-shrink-0", | ||
| this.disabled() ? "tw-text-fg-inactive" : "tw-text-fg-heading", | ||
| ].join(" "), | ||
| ); | ||
|
|
||
| protected readonly contentClassList = computed(() => | ||
| [ | ||
| "tw-p-4", | ||
| "tw-rounded-b-xl", | ||
| this.resolvedVariant() === "subtle" | ||
| ? "tw-border-t tw-border-solid tw-border-border-base" | ||
| : "", | ||
| ].join(" "), | ||
| ); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there any typography variants that we can use for heading and subtitle?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes and no. Because this switches text sizes and colors based on inactive state and the size variant passed in this felt easier to swap