Skip to content

feat: ui components, tanstack query setup#118

Merged
ludavidca merged 13 commits into
mainfrom
feat/ui-components
Apr 18, 2026
Merged

feat: ui components, tanstack query setup#118
ludavidca merged 13 commits into
mainfrom
feat/ui-components

Conversation

@landont168

@landont168 landont168 commented Apr 9, 2026

Copy link
Copy Markdown
Member
  • added Banner, Tag, Card, DataTable, DropdownTable components
  • installed and setup TanStack Query
  • installed XState React

How to use TanStack Query

  • Create a hook in src/api/
  • Reexport in barrel file src/api/index.ts
import { useDrivers } from '@/api';                                             
const { data, isPending } = useDrivers();  

UI COMPONENTS
image
image
image
image
image

@ludavidca

Copy link
Copy Markdown
Collaborator

@claude review

@uwblueprint uwblueprint deleted a comment from landont168 Apr 9, 2026
@claude

claude Bot commented Apr 9, 2026

Copy link
Copy Markdown

PR Review: feat/ui-components — UI Components + TanStack Query Setup

Overall this is a solid PR. The components are well-typed, consistently use CVA + cn(), and the TanStack Query setup is straightforward. A few things worth addressing before merging:


Bugs / Correctness

1. ReactQueryDevtools ships to production

main.tsx renders <ReactQueryDevtools> unconditionally. This bundles the devtools into the production build. Wrap it in a dev guard:

// main.tsx
{import.meta.env.DEV && <ReactQueryDevtools initialIsOpen={false} />}

2. DropdownTable uses array index as React key

// DropdownTable.tsx line ~59
{rows.map((row, i) => (
  <div key={i} ...>

Index keys cause incorrect reconciliation when rows are reordered or filtered. Use row.label (which appears to be unique in practice) or add an explicit id field to DropdownTableRow:

<div key={row.label} ...>

Dead Code

3. DataTableSection.tsx and DropdownTableSection.tsx are never imported

StyleGuide/index.tsx imports TableSection and TagsSection but not these two files. They currently have no consumers. Either delete them or wire them up — right now they're just orphaned files with duplicate mock data.

4. TableSection.tsx duplicates mock data already in DataTableSection.tsx

Both files define identical MockRow, MOCK_ROWS, and column configs. If both sections are eventually kept, extract the shared mock data to a __fixtures__ or mockData.ts file alongside the style guide.


Minor Issues

5. Misleading comment in axiosClient.ts

// Global response error handling (e.g. redirect on 401)
axiosClient.interceptors.response.use(
  (response) => response,
  (error) => {
    return Promise.reject(error);  // ← does nothing
  }
);

The comment promises a 401 redirect but the handler just re-throws. Either implement it or remove the interceptor (and the misleading comment) until it's needed.

6. Banner variant prop is required but CVA has a default

// Banner.tsx
interface BannerProps ... {
  variant: BannerVariant;  // required
}

The CVA defaultVariants: { variant: 'success' } is never reached because callers are forced to pass variant. Make it optional (variant?: BannerVariant) to let the default actually do its job.

7. Card padding can be simplified

'pt-6 pr-8 pb-8 pl-8'
// → 'p-8 pt-6'

Question

8. XState added but not used

xstate and @xstate/react appear in package.json but nothing in this PR uses them. Is this intentional setup for a follow-on PR? If so, a note in the PR description would help reviewers understand why it's here. If not, consider deferring the install until it's actually needed.


The core components (Banner, Card, DataTable, DropdownTable, Tag) look good — clean API design and proper TypeScript generics in DataTable. Issues 1 and 2 are the ones most worth fixing before merge.

Comment thread frontend/src/lib/axiosClient.ts

@ludavidca ludavidca left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small Comment. Approved!

@ludavidca

Copy link
Copy Markdown
Collaborator

Merged in now due to Axios blocking Eddy's Ticket At the Moment.

@ludavidca
ludavidca merged commit 95d401b into main Apr 18, 2026
3 checks passed
@landont168
landont168 deleted the feat/ui-components branch April 27, 2026 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants