Fix UI: deduplicate DataTable, abstract localStorage, remove admin placeholder by intel352 · Pull Request #13 · GoCodeAlone/workflow-ui

intel352 · 2026-02-26T20:51:17Z

Summary

Remove duplicate flat src/components/DataTable.tsx implementation; canonical version lives in src/components/DataTable/DataTable.tsx
Remove hardcoded admin placeholder from LoginPage component
Abstract localStorage access in authStore for SSR compatibility and testability
Fix components/index.ts and src/index.ts to use correct named re-exports and remove non-existent SortState/SortDirection type exports that caused build failure
Fix DataTable.test.tsx button queries to match component's aria-label values ('Next page'/'Previous page')

Test plan

npm run build passes (vite build + tsc declaration emit)
npm test — 144/144 tests pass across 15 test files
No TypeScript errors

🤖 Generated with Claude Code

…adingSpinner, ErrorBoundary + accessibility (#3, #4) Add 5 new shared React components with full TypeScript types, unit tests, and accessibility support: - StatusBadge: color-coded workflow status indicator - DataTable: sortable, paginated data table - Modal: confirmation/error/info dialogs - LoadingSpinner: loading state indicator - ErrorBoundary: React error boundary wrapper All components use semantic HTML, ARIA labels, and support keyboard navigation (WCAG compliance). Closes #3, Closes #4 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Initial plan * fix: address PR review comments for DataTable, Modal, and ErrorBoundary Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>

Resolve merge conflicts in component index exports (use default exports and correct type names from main). Fix 3 LoginPage test assertions that didn't match the component implementation. Downgrade jsdom from 28 to 26.1.0 to fix ESM incompatibility with @exodus/bytes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- DataTable: type key as keyof T, import ReactNode, remove role="grid", clamp pagination, memoize sorting - Modal: fix double-escape, import event types, use useId for title - ErrorBoundary: remove mismatched aria-label on retry button - ErrorBoundary tests: import beforeEach/afterEach from vitest - LoadingSpinner: move keyframe injection to useEffect - StatusBadge: use strict types for size style records Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Replace useEffect + setState pagination clamping with derived safePage value to satisfy react-hooks/set-state-in-effect lint rule. The flat DataTable already had safePage derived inline; remove the redundant effect. The directory-based DataTable now derives safePage from the raw page state and totalPages. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Remove duplicate 'has accessible form with aria-label' test that was identical to 'renders a <form> element for the login form'. Switch LoadingSpinner keyframe injection from useLayoutEffect to useEffect to avoid SSR warnings in Next.js/Remix consumers. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Delete the flat src/components/DataTable.tsx file (the stale implementation) and update the test file to use the subdirectory version at src/components/DataTable/DataTable.tsx. Changed column definitions from 'label' to 'header', added required 'rowKey' prop, and updated test assertions to match the subdirectory component's API. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Changed the default username placeholder from 'admin' to 'Enter username' for better UX and to avoid hardcoding test credentials in the UI. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Added StorageAdapter interface and storage configuration option to AuthStoreConfig. The store now accepts an optional storage adapter (defaults to localStorage) allowing for testable storage behavior and SSR compatibility. All localStorage calls now use the configurable storage instance. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…-exports - Use named re-export from DataTable/index.ts (not default) in components/index.ts - Remove non-existent SortState and SortDirection from components/index.ts and src/index.ts - Fix DataTable.test.tsx button queries to use 'Next page'/'Previous page' matching aria-labels Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Confirms deletion of stale src/components/DataTable.tsx — canonical implementation is in src/components/DataTable/DataTable.tsx. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

This PR focuses on code quality improvements by deduplicating components, improving abstractions, and fixing build issues.

Changes:

Removed duplicate DataTable implementation and consolidated to the subdirectory version
Abstracted localStorage access in authStore via StorageAdapter interface for SSR compatibility
Removed hardcoded 'admin' placeholder from LoginPage, replacing with 'Enter username'
Reorganized components (StatusBadge, Modal, LoadingSpinner, ErrorBoundary, DataTable) into subdirectories with proper index.ts files
Fixed LoadingSpinner to wrap keyframe injection in useEffect for proper lifecycle management
Corrected package.json jsdom version and removed non-existent type exports

Reviewed changes

Copilot reviewed 79 out of 137 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/components/DataTable.tsx	Removed duplicate flat implementation
src/components/DataTable/	Canonical DataTable implementation with tests
src/auth/authStore.ts	Added StorageAdapter interface, defaulting to localStorage
src/auth/LoginPage.tsx	Changed placeholder from 'admin' to 'Enter username'
src/components/StatusBadge/	Moved to subdirectory structure
src/components/Modal/	Moved to subdirectory with comprehensive tests
src/components/LoadingSpinner/	Moved to subdirectory, wrapped keyframe injection in useEffect
src/components/ErrorBoundary/	Moved to subdirectory with tests
src/components/index.ts	Updated exports to use subdirectory structure
src/index.ts	Removed non-existent SortState/SortDirection exports
package.json	Fixed jsdom version from invalid ^28.0.0 to ^26.1.0

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-26T20:54:15Z

src/components/StatusBadge/index.ts

@@ -0,0 +1,2 @@
+export { default as StatusBadge } from './StatusBadge';
+export type { StatusBadgeProps, StatusType } from './StatusBadge';


The StatusBadge/index.ts exports StatusType but the main src/index.ts and src/components/index.ts still export StatusValue and StatusBadgeSize. The new StatusBadge component uses StatusType instead of StatusValue, and StatusBadgeSize is now an internal type. This creates an API breaking change where consumers importing StatusValue will get an error.

Either:

Export StatusType as StatusValue for backward compatibility: export type { StatusType as StatusValue }

Or update src/index.ts to export StatusType instead of StatusValue

Copilot · 2026-02-26T20:54:15Z

src/components/StatusBadge/index.ts

@@ -0,0 +1,2 @@
+export { default as StatusBadge } from './StatusBadge';
+export type { StatusBadgeProps, StatusType } from './StatusBadge';


The StatusBadgeSize type is no longer exported but src/index.ts line 36 still tries to export it. This will cause a build failure since StatusBadgeSize is now an internal type in StatusBadge/StatusBadge.tsx (line 25). Either export StatusBadgeSize from StatusBadge/index.ts or remove it from the public API exports in src/index.ts and src/components/index.ts.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 61 out of 118 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-26T21:00:36Z

src/auth/authStore.ts

+  const storage = config?.storage ?? localStorage;

  return create<BaseAuthState>((set) => ({
-    token: localStorage.getItem(tokenKey),
+    token: storage.getItem(tokenKey),
    user: null,


const storage = config?.storage ?? localStorage; will still throw a ReferenceError in SSR/Node environments where localStorage is not defined. If SSR compatibility is a goal, guard the default with typeof localStorage !== 'undefined' (or typeof window !== 'undefined' && window.localStorage) and fall back to a no-op/in-memory StorageAdapter, or require callers to pass config.storage when localStorage is unavailable.

Copilot · 2026-02-26T21:00:37Z

src/components/DataTable.test.tsx

+    render(<DataTable columns={columns} data={data} pageSize={10} rowKey={(row) => String(row.id)} />);
+    expect(screen.queryByRole('button', { name: 'Next' })).not.toBeInTheDocument();
  });


This assertion uses { name: 'Next' }, but the component’s pagination button has aria-label="Next page" (accessible name is "Next page"). As written, the test will pass even if the Next button is present, so it won’t actually verify pagination controls are hidden for a single page. Query for the correct accessible name (or for the navigation element) to make this test meaningful.

Copilot · 2026-02-26T21:00:37Z

.playwright-cli/console-2026-02-24T09-46-29-209Z.log

+[VERBOSE] [DOM] Input elements should have autocomplete attributes (suggested: "new-password"): (More info: https://goo.gl/9p2vKq) %o @ http://localhost:8081/:0
+[VERBOSE] [DOM] Input elements should have autocomplete attributes (suggested: "new-password"): (More info: https://goo.gl/9p2vKq) %o @ http://localhost:8081/:0
+[ERROR] Failed to load resource: the server responded with a status of 404 (Not Found) @ http://localhost:8081/api/v1/admin/iam/providers/00000000-0000-0000-0000-000000000001:0
+[ERROR] Failed to load resource: the server responded with a status of 401 (Unauthorized) @ http://localhost:8081/api/v1/admin/feature-flags/stream?token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJlbWFpbCI6ImFkbWluQHRlc3QuY29tIiwiZXhwIjoxNzcyMDEyMzkzLCJpYXQiOjE3NzE5MjU5OTMsImlzcyI6IndvcmtmbG93LWFkbWluIiwibmFtZSI6IkFkbWluIiwicm9sZSI6ImFkbWluIiwic3ViIjoiMSJ9.SHPSwA8kI8aSOunbhv7i3S5IqwEs6-DRQE6BznIzBeQ:0


This console log file contains what appears to be a real JWT/auth token embedded in a URL query parameter. Committing tokens to the repo is a security risk (credentials leakage) and these .playwright-cli run artifacts (logs/snapshots) should generally not be versioned. Remove these artifacts from the PR and add .playwright-cli/ (and similar output) to .gitignore; also rotate/revoke any exposed tokens.

Suggested change

[ERROR] Failed to load resource: the server responded with a status of 401 (Unauthorized) @ http://localhost:8081/api/v1/admin/feature-flags/stream?token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJlbWFpbCI6ImFkbWluQHRlc3QuY29tIiwiZXhwIjoxNzcyMDEyMzkzLCJpYXQiOjE3NzE5MjU5OTMsImlzcyI6IndvcmtmbG93LWFkbWluIiwibmFtZSI6IkFkbWluIiwicm9sZSI6ImFkbWluIiwic3ViIjoiMSJ9.SHPSwA8kI8aSOunbhv7i3S5IqwEs6-DRQE6BznIzBeQ:0

[ERROR] Failed to load resource: the server responded with a status of 401 (Unauthorized) @ http://localhost:8081/api/v1/admin/feature-flags/stream?token=<REDACTED>:0

intel352 and others added 11 commits February 23, 2026 05:54

Merge branch 'main' into feat/issue-3-4-components-a11y

99e02f7

Fix 2: Remove hardcoded 'admin' placeholder from LoginPage

7c4f2f5

Changed the default username placeholder from 'admin' to 'Enter username' for better UX and to avoid hardcoding test credentials in the UI. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings February 26, 2026 20:51

Copilot started reviewing on behalf of intel352 February 26, 2026 20:51 View session

Merge main into fix/ws12-ui-fixes: resolve DataTable.tsx delete conflict

ee42d29

Confirms deletion of stale src/components/DataTable.tsx — canonical implementation is in src/components/DataTable/DataTable.tsx. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot AI reviewed Feb 26, 2026

View reviewed changes

Fix lint: remove unused 'vi' import from DataTable.test.tsx

8205f03

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings February 26, 2026 20:56

Copilot started reviewing on behalf of intel352 February 26, 2026 20:56 View session

intel352 merged commit b499d06 into main Feb 26, 2026
5 checks passed

intel352 deleted the fix/ws12-ui-fixes branch February 26, 2026 20:57

Copilot AI reviewed Feb 26, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix UI: deduplicate DataTable, abstract localStorage, remove admin placeholder#13

Fix UI: deduplicate DataTable, abstract localStorage, remove admin placeholder#13
intel352 merged 13 commits intomainfrom
fix/ws12-ui-fixes

intel352 commented Feb 26, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 26, 2026

Uh oh!

Copilot AI Feb 26, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 26, 2026

Uh oh!

Copilot AI Feb 26, 2026

Uh oh!

Copilot AI Feb 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		@@ -0,0 +1,2 @@
		export { default as StatusBadge } from './StatusBadge';
		export type { StatusBadgeProps, StatusType } from './StatusBadge';

Conversation

intel352 commented Feb 26, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants