From a061e68bde3adffc7243ed2b340b542e0603010d Mon Sep 17 00:00:00 2001 From: "Frank Pigeon Jr." <4629398+fpigeonjr@users.noreply.github.com> Date: Fri, 28 Aug 2026 11:51:48 -0500 Subject: [PATCH 1/2] Add specs for modal, image, and data-table sort components - Un-skip modal.spec.ts (pre-existing tests already pass under Vitest) and add specs for reopen guard, closeModal(false), Escape-key close/no-close branches, modalElIds construction, selectedType class, and the Tab focus-trap keydown listener. - Add specs for image.component.ts covering ngOnInit's file-picker, save/cancel, and drag-and-drop wiring. - Add sort.directive.spec.ts covering register/deregister, duplicate and missing id errors, and sort direction cycling with disableClear overrides. Coverage: modal.component.ts 37.14% -> 99.04%, image.component.ts 45.61% -> 92.98%, sort.directive.ts 28.12% -> 100% (line coverage). Closes #632 --- .../data-table/sort.directive.spec.ts | 146 +++++++++++++++ src/ui-kit/components/image/image.spec.ts | 104 ++++++++++- src/ui-kit/components/modal/modal.spec.ts | 175 +++++++++++++++++- 3 files changed, 417 insertions(+), 8 deletions(-) create mode 100644 src/ui-kit/components/data-table/sort.directive.spec.ts diff --git a/src/ui-kit/components/data-table/sort.directive.spec.ts b/src/ui-kit/components/data-table/sort.directive.spec.ts new file mode 100644 index 000000000..895383d88 --- /dev/null +++ b/src/ui-kit/components/data-table/sort.directive.spec.ts @@ -0,0 +1,146 @@ +import { SamSortDirective, SamSortable } from "./sort.directive"; + +function makeSortable(overrides: Partial = {}): SamSortable { + return { + id: "col-a", + start: undefined as unknown as SamSortable["start"], + disableClear: undefined as unknown as SamSortable["disableClear"], + ...overrides, + }; +} + +describe("The Sam Sort directive", () => { + let directive: SamSortDirective; + + beforeEach(() => { + directive = new SamSortDirective(); + }); + + it("registers a sortable by id", () => { + const sortable = makeSortable({ id: "col-a" }); + directive.register(sortable); + + expect(directive.sortables.get("col-a")).toBe(sortable); + }); + + it("throws when registering a sortable with no id", () => { + const sortable = makeSortable({ id: "" }); + + expect(() => directive.register(sortable)).toThrow( + "Missing Sort Header ID error" + ); + }); + + it("throws when registering a duplicate id", () => { + directive.register(makeSortable({ id: "col-a" })); + + expect(() => directive.register(makeSortable({ id: "col-a" }))).toThrow( + "Duplicate Sort Header ID error: col-a" + ); + }); + + it("deregisters a sortable by id", () => { + const sortable = makeSortable({ id: "col-a" }); + directive.register(sortable); + directive.deregister(sortable); + + expect(directive.sortables.has("col-a")).toBe(false); + }); + + it("activates a new sortable using its own start direction", () => { + const sortable = makeSortable({ id: "col-a", start: "desc" }); + + directive.sort(sortable); + + expect(directive.active).toBe("col-a"); + expect(directive.direction).toBe("desc"); + }); + + it("falls back to the directive's start direction when the sortable has none", () => { + directive.start = "desc"; + const sortable = makeSortable({ id: "col-a", start: undefined }); + + directive.sort(sortable); + + expect(directive.direction).toBe("desc"); + }); + + it("emits samSortChange with the active id and direction", () => { + const sortable = makeSortable({ id: "col-a", start: "asc" }); + let emitted: any; + directive.samSortChange.subscribe((val) => { + emitted = val; + }); + + directive.sort(sortable); + + expect(emitted).toEqual({ active: "col-a", direction: "asc" }); + }); + + it("cycles asc -> desc -> clear on repeated sorts of the same sortable", () => { + const sortable = makeSortable({ id: "col-a", start: "asc" }); + + directive.sort(sortable); + expect(directive.direction).toBe("asc"); + + directive.sort(sortable); + expect(directive.direction).toBe("desc"); + + directive.sort(sortable); + expect(directive.direction).toBe(""); + + directive.sort(sortable); + expect(directive.direction).toBe("asc"); + }); + + it("cycles desc -> asc -> clear when start is desc", () => { + const sortable = makeSortable({ id: "col-a", start: "desc" }); + + directive.sort(sortable); + expect(directive.direction).toBe("desc"); + + directive.sort(sortable); + expect(directive.direction).toBe("asc"); + + directive.sort(sortable); + expect(directive.direction).toBe(""); + }); + + it("skips the clear step when disableClear is set on the directive", () => { + directive.disableClear = true; + const sortable = makeSortable({ id: "col-a", start: "asc" }); + + directive.sort(sortable); + expect(directive.direction).toBe("asc"); + + directive.sort(sortable); + expect(directive.direction).toBe("desc"); + + directive.sort(sortable); + expect(directive.direction).toBe("asc"); + }); + + it("lets a sortable's own disableClear override the directive's setting", () => { + directive.disableClear = false; + const sortable = makeSortable({ + id: "col-a", + start: "asc", + disableClear: true, + }); + + directive.sort(sortable); + expect(directive.direction).toBe("asc"); + + directive.sort(sortable); + expect(directive.direction).toBe("desc"); + + directive.sort(sortable); + expect(directive.direction).toBe("asc"); + }); + + it("returns an empty direction from getNextSortDirection when no sortable is given", () => { + expect( + directive.getNextSortDirection(undefined as unknown as SamSortable) + ).toBe(""); + }); +}); diff --git a/src/ui-kit/components/image/image.spec.ts b/src/ui-kit/components/image/image.spec.ts index cda1ceeaa..79b3858dd 100755 --- a/src/ui-kit/components/image/image.spec.ts +++ b/src/ui-kit/components/image/image.spec.ts @@ -16,16 +16,12 @@ describe("The Sam Image Component", () => { TestBed.configureTestingModule({ declarations: [SamImageComponent], }).compileComponents(); - let emittedFile: File; fixture = TestBed.createComponent(SamImageComponent); component = fixture.componentInstance; de = fixture.debugElement; component.editable = true; - component.fileChange.subscribe((file: File) => { - emittedFile = file; - }); component.src = washingtonImg; fixture.detectChanges(); }); @@ -49,4 +45,104 @@ describe("The Sam Image Component", () => { ).nativeElement; expect(buttonEl.disabled).toBe(true); }); + + it("uploads a file, sets tmp state, and emits fileChange on save", async () => { + const file = new File(["hello"], "washington.png", { type: "image/png" }); + const fileInputEl: HTMLInputElement = de.query( + By.css("input#file") + ).nativeElement; + Object.defineProperty(fileInputEl, "files", { value: [file] }); + fileInputEl.dispatchEvent(new Event("change")); + + await new Promise((resolve) => setTimeout(resolve, 10)); + + expect(component.getFileName()).toBe("washington.png"); + + let emittedFile: File | undefined; + component.fileChange.subscribe((f) => { + emittedFile = f; + }); + + const saveButtonEl: HTMLButtonElement = de.query( + By.css("button.save-button") + ).nativeElement; + saveButtonEl.dispatchEvent(new Event("click")); + + expect(emittedFile).toBe(file); + expect(component.src).toBeDefined(); + }); + + it("clears tmp file state when cancel is clicked", async () => { + const file = new File(["hello"], "washington.png", { type: "image/png" }); + const fileInputEl: HTMLInputElement = de.query( + By.css("input#file") + ).nativeElement; + Object.defineProperty(fileInputEl, "files", { value: [file] }); + fileInputEl.dispatchEvent(new Event("change")); + + await new Promise((resolve) => setTimeout(resolve, 10)); + expect(component.getFileName()).toBe("washington.png"); + + const cancelButtonEl: HTMLButtonElement = de.query( + By.css("button.cancel-button") + ).nativeElement; + cancelButtonEl.dispatchEvent(new Event("click")); + + expect(component.getFileName()).toBe(""); + expect(component.isImageTemporary()).toBe(false); + }); + + it("drops a file onto the container when in edit mode", async () => { + component.editMode = true; + const file = new File(["hello"], "dropped.png", { type: "image/png" }); + + const containerEl: HTMLElement = de.query( + By.css("div.sam-image") + ).nativeElement; + const dropEvent = new Event("drop", { + bubbles: true, + cancelable: true, + }) as Event & { dataTransfer: { files: File[] } }; + dropEvent.dataTransfer = { files: [file] }; + containerEl.dispatchEvent(dropEvent); + + await new Promise((resolve) => setTimeout(resolve, 10)); + + expect(component.getFileName()).toBe("dropped.png"); + }); + + it("ignores a drop when not in edit mode", () => { + component.editMode = false; + const file = new File(["hello"], "dropped.png", { type: "image/png" }); + + const containerEl: HTMLElement = de.query( + By.css("div.sam-image") + ).nativeElement; + const dropEvent = new Event("drop", { + bubbles: true, + cancelable: true, + }) as Event & { dataTransfer: { files: File[] } }; + dropEvent.dataTransfer = { files: [file] }; + containerEl.dispatchEvent(dropEvent); + + expect(component.getFileName()).toBe(""); + }); + + it("stops propagation and prevents default on drag enter/over", () => { + const enterEvent = { + stopPropagation: vi.fn(), + preventDefault: vi.fn(), + }; + component.onDragEnter(enterEvent as unknown as DragEvent); + expect(enterEvent.stopPropagation).toHaveBeenCalled(); + expect(enterEvent.preventDefault).toHaveBeenCalled(); + + const overEvent = { + stopPropagation: vi.fn(), + preventDefault: vi.fn(), + }; + component.onDragOver(overEvent as unknown as DragEvent); + expect(overEvent.stopPropagation).toHaveBeenCalled(); + expect(overEvent.preventDefault).toHaveBeenCalled(); + }); }); diff --git a/src/ui-kit/components/modal/modal.spec.ts b/src/ui-kit/components/modal/modal.spec.ts index ed1f55642..2612b1117 100755 --- a/src/ui-kit/components/modal/modal.spec.ts +++ b/src/ui-kit/components/modal/modal.spec.ts @@ -1,13 +1,12 @@ -import { TestBed, waitForAsync } from "@angular/core/testing"; +import { ComponentFixture, TestBed } from "@angular/core/testing"; import { By } from "@angular/platform-browser"; -import { ElementRef } from "@angular/core"; // Load the implementations that should be tested import { SamModalComponent } from "./modal.component"; import { SamElementsModule } from "../../elements"; -describe.skip("The Sam Modal component", () => { +describe("The Sam Modal component", () => { describe("isolated tests", () => { let component: SamModalComponent; beforeEach(() => { @@ -31,7 +30,7 @@ describe.skip("The Sam Modal component", () => { describe("rendered tests", () => { let component: SamModalComponent; - let fixture: any; + let fixture: ComponentFixture; beforeEach(() => { TestBed.configureTestingModule({ @@ -60,5 +59,173 @@ describe.skip("The Sam Modal component", () => { }); component.closeModal(); }); + + it("should not reopen or re-emit open when already shown", () => { + component.type = "success"; + component.openModal("first"); + fixture.detectChanges(); + + let openEmitCount = 0; + component.open.subscribe(() => { + openEmitCount++; + }); + component.openModal("second"); + + expect(openEmitCount).toBe(0); + }); + + it("should not emit close when closeModal is called with emit=false", () => { + component.type = "success"; + component.openModal("test"); + fixture.detectChanges(); + + let closeEmitCount = 0; + component.close.subscribe(() => { + closeEmitCount++; + }); + component.closeModal(false); + + expect(closeEmitCount).toBe(0); + expect(component.show).toBe(false); + }); + + it("should close on Escape key when closeOnEscape is true", () => { + component.type = "success"; + component.closeOnEscape = true; + component.openModal("test"); + fixture.detectChanges(); + + component.closeEscape({ + keyCode: 27, + stopPropagation: () => {}, + } as unknown as KeyboardEvent); + + expect(component.show).toBe(false); + }); + + it("should not close on Escape key when closeOnEscape is false", () => { + component.type = "success"; + component.closeOnEscape = false; + component.openModal("test"); + fixture.detectChanges(); + + component.closeEscape({ + keyCode: 27, + stopPropagation: () => {}, + } as unknown as KeyboardEvent); + + expect(component.show).toBe(true); + }); + + it("should not close on non-Escape keys", () => { + component.type = "success"; + component.closeOnEscape = true; + component.openModal("test"); + fixture.detectChanges(); + + component.closeEscape({ + keyCode: 13, + stopPropagation: () => {}, + } as unknown as KeyboardEvent); + + expect(component.show).toBe(true); + }); + + it("should build modal element ids from the id input on init", () => { + const freshFixture = TestBed.createComponent(SamModalComponent); + const freshComponent = freshFixture.componentInstance; + freshComponent.id = "my-modal"; + freshComponent.ngOnInit(); + + expect(freshComponent.modalElIds).toEqual({ + cancelId: "my-modalCancel", + closeId: "my-modalClose", + submitId: "my-modalSubmit", + }); + }); + + it("should apply the type's alert class when a known type is set before init", () => { + const freshFixture = TestBed.createComponent(SamModalComponent); + const freshComponent = freshFixture.componentInstance; + freshComponent.type = "warning"; + freshComponent.ngOnInit(); + + expect(freshComponent.selectedType).toBe("usa-alert-warning"); + }); + + it("should trap Tab focus between the first and last focusable buttons", () => { + component.type = "success"; + component.cancelButtonLabel = "Cancel"; + component.submitButtonLabel = "Submit"; + component.openModal("test"); + fixture.detectChanges(); + + const modalContentEl = fixture.debugElement.query( + By.css(".modal-content") + ).nativeElement as HTMLElement; + const buttons = fixture.debugElement.queryAll(By.css("button")); + const firstFocusEl = buttons[0].nativeElement as HTMLButtonElement; + const lastFocusEl = buttons[buttons.length - 1] + .nativeElement as HTMLButtonElement; + + const firstFocusSpy = vi.spyOn(firstFocusEl, "focus"); + modalContentEl.dispatchEvent( + new KeyboardEvent("keydown", { keyCode: 9, shiftKey: false }) + ); + expect(firstFocusSpy).toHaveBeenCalled(); + + const lastFocusSpy = vi.spyOn(lastFocusEl, "focus"); + firstFocusEl.dispatchEvent( + new KeyboardEvent("keydown", { keyCode: 9, shiftKey: true }) + ); + expect(lastFocusSpy).toHaveBeenCalled(); + + const firstFocusSpyAgain = vi.spyOn(firstFocusEl, "focus"); + lastFocusEl.dispatchEvent( + new KeyboardEvent("keydown", { keyCode: 9, shiftKey: false }) + ); + expect(firstFocusSpyAgain).toHaveBeenCalled(); + }); + + it("should shift+tab from the modal container to the last focusable button", () => { + component.type = "success"; + component.cancelButtonLabel = "Cancel"; + component.submitButtonLabel = "Submit"; + component.openModal("test"); + fixture.detectChanges(); + + const modalContentEl = fixture.debugElement.query( + By.css(".modal-content") + ).nativeElement as HTMLElement; + const buttons = fixture.debugElement.queryAll(By.css("button")); + const lastFocusEl = buttons[buttons.length - 1] + .nativeElement as HTMLButtonElement; + + const lastFocusSpy = vi.spyOn(lastFocusEl, "focus"); + modalContentEl.dispatchEvent( + new KeyboardEvent("keydown", { keyCode: 9, shiftKey: true }) + ); + expect(lastFocusSpy).toHaveBeenCalled(); + }); + + it("should prevent Tab from leaving the modal when there is only one focusable element", () => { + component.type = "success"; + component.showClose = false; + component.openModal("test"); + fixture.detectChanges(); + + const modalContentEl = fixture.debugElement.query( + By.css(".modal-content") + ).nativeElement as HTMLElement; + + const event = new KeyboardEvent("keydown", { + keyCode: 9, + shiftKey: false, + }); + const preventDefaultSpy = vi.spyOn(event, "preventDefault"); + modalContentEl.dispatchEvent(event); + + expect(preventDefaultSpy).toHaveBeenCalled(); + }); }); }); From 8816b375aa44c308f32414ac97165710d0461f20 Mon Sep 17 00:00:00 2001 From: "Frank Pigeon Jr." <4629398+fpigeonjr@users.noreply.github.com> Date: Fri, 28 Aug 2026 13:14:29 -0500 Subject: [PATCH 2/2] Address PR review feedback - Add an afterEach in modal.spec.ts's rendered-tests block to close the modal (or ngOnDestroy) and remove the modal-open body class, preventing leaked global state between tests. - Replace image.spec.ts's setTimeout(10) waits with a FileReader.prototype.readAsDataURL spy that fires onload synchronously, so the file-upload and drag-and-drop specs are deterministic instead of timing-dependent. --- src/ui-kit/components/image/image.spec.ts | 24 +++++++++++++++-------- src/ui-kit/components/modal/modal.spec.ts | 8 ++++++++ 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/ui-kit/components/image/image.spec.ts b/src/ui-kit/components/image/image.spec.ts index 79b3858dd..d53e7a5f3 100755 --- a/src/ui-kit/components/image/image.spec.ts +++ b/src/ui-kit/components/image/image.spec.ts @@ -11,8 +11,17 @@ describe("The Sam Image Component", () => { let fixture: ComponentFixture; let component: SamImageComponent; let de: DebugElement; + let readAsDataURLSpy: ReturnType; beforeEach(() => { + readAsDataURLSpy = vi + .spyOn(FileReader.prototype, "readAsDataURL") + .mockImplementation(function (this: FileReader, file: Blob) { + this.onload?.({ + target: { result: `data:fake;name=${(file as File).name}` }, + } as unknown as ProgressEvent); + }); + TestBed.configureTestingModule({ declarations: [SamImageComponent], }).compileComponents(); @@ -26,6 +35,10 @@ describe("The Sam Image Component", () => { fixture.detectChanges(); }); + afterEach(() => { + readAsDataURLSpy.mockRestore(); + }); + it("has an edit button that opens file upload", () => { const buttonEl: DebugElement = de.query(By.css("button.edit-button")); @@ -46,7 +59,7 @@ describe("The Sam Image Component", () => { expect(buttonEl.disabled).toBe(true); }); - it("uploads a file, sets tmp state, and emits fileChange on save", async () => { + it("uploads a file, sets tmp state, and emits fileChange on save", () => { const file = new File(["hello"], "washington.png", { type: "image/png" }); const fileInputEl: HTMLInputElement = de.query( By.css("input#file") @@ -54,8 +67,6 @@ describe("The Sam Image Component", () => { Object.defineProperty(fileInputEl, "files", { value: [file] }); fileInputEl.dispatchEvent(new Event("change")); - await new Promise((resolve) => setTimeout(resolve, 10)); - expect(component.getFileName()).toBe("washington.png"); let emittedFile: File | undefined; @@ -72,7 +83,7 @@ describe("The Sam Image Component", () => { expect(component.src).toBeDefined(); }); - it("clears tmp file state when cancel is clicked", async () => { + it("clears tmp file state when cancel is clicked", () => { const file = new File(["hello"], "washington.png", { type: "image/png" }); const fileInputEl: HTMLInputElement = de.query( By.css("input#file") @@ -80,7 +91,6 @@ describe("The Sam Image Component", () => { Object.defineProperty(fileInputEl, "files", { value: [file] }); fileInputEl.dispatchEvent(new Event("change")); - await new Promise((resolve) => setTimeout(resolve, 10)); expect(component.getFileName()).toBe("washington.png"); const cancelButtonEl: HTMLButtonElement = de.query( @@ -92,7 +102,7 @@ describe("The Sam Image Component", () => { expect(component.isImageTemporary()).toBe(false); }); - it("drops a file onto the container when in edit mode", async () => { + it("drops a file onto the container when in edit mode", () => { component.editMode = true; const file = new File(["hello"], "dropped.png", { type: "image/png" }); @@ -106,8 +116,6 @@ describe("The Sam Image Component", () => { dropEvent.dataTransfer = { files: [file] }; containerEl.dispatchEvent(dropEvent); - await new Promise((resolve) => setTimeout(resolve, 10)); - expect(component.getFileName()).toBe("dropped.png"); }); diff --git a/src/ui-kit/components/modal/modal.spec.ts b/src/ui-kit/components/modal/modal.spec.ts index 2612b1117..720dd53e2 100755 --- a/src/ui-kit/components/modal/modal.spec.ts +++ b/src/ui-kit/components/modal/modal.spec.ts @@ -44,6 +44,14 @@ describe("The Sam Modal component", () => { fixture.detectChanges(); }); + afterEach(() => { + if (component.show) { + component.closeModal(false); + } + component.ngOnDestroy(); + document.body.classList.remove("modal-open"); + }); + it("should open and close modal", function () { component.title = "test title"; component.type = "success";