-
Notifications
You must be signed in to change notification settings - Fork 0
feat(@helm/web): multi-product dashboard with tabs and URL routing (Session 8) #13
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
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
3a33f2b
feat(@helm/web): add multi-product API methods to client
lhpaul 066fa95
feat(@helm/web): add ProductTabs component with 5s polling
lhpaul 6fff7ba
feat(@helm/web): add URL-based routing and refactor views for multi-p…
lhpaul 98f37bc
test(@helm/web): add Kanban + App routing tests, fix useMemo hook order
lhpaul 55d3501
fix(@helm/web): address 7 CodeRabbit findings on PR #13
lhpaul 2589741
fix(@helm/web): address 3 more CodeRabbit findings on PR #13
lhpaul File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| import { render, screen } from '@testing-library/react'; | ||
| import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; | ||
| import App from './App.js'; | ||
|
|
||
| // ── Mocks ───────────────────────────────────────────────────────────────────── | ||
|
|
||
| const { mockListProducts, mockGetProductBySlug, mockListItemsForProduct } = vi.hoisted(() => ({ | ||
| mockListProducts: vi.fn(), | ||
| mockGetProductBySlug: vi.fn(), | ||
| mockListItemsForProduct: vi.fn(), | ||
| })); | ||
|
|
||
| vi.mock('./lib/api.js', async (importOriginal) => { | ||
| const real = await importOriginal<typeof import('./lib/api.js')>(); | ||
| return { | ||
| ...real, | ||
| api: { | ||
| ...real.api, | ||
| listProducts: mockListProducts, | ||
| getProductBySlug: mockGetProductBySlug, | ||
| listItemsForProduct: mockListItemsForProduct, | ||
| }, | ||
| }; | ||
| }); | ||
|
|
||
| // ── Helpers ─────────────────────────────────────────────────────────────────── | ||
|
|
||
| function ok<T>(data: T) { | ||
| const safeData = | ||
| data && typeof data === 'object' | ||
| ? Array.isArray(data) | ||
| ? ([...data] as T) | ||
| : ({ ...data } as T) | ||
| : data; | ||
| return { ok: true as const, data: safeData }; | ||
| } | ||
|
|
||
| const PRODUCTS = [ | ||
| { product: { slug: 'helm', name: 'Helm' }, workflow: { stages_enabled: ['discovery'] as const } }, | ||
| ]; | ||
|
|
||
| // ── Tests ───────────────────────────────────────────────────────────────────── | ||
|
|
||
| describe('App routing', () => { | ||
| beforeEach(() => { | ||
| vi.useFakeTimers(); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| vi.useRealTimers(); | ||
| vi.restoreAllMocks(); | ||
| }); | ||
|
|
||
| it('shows empty state when registry returns no products', async () => { | ||
| mockListProducts.mockResolvedValue(ok([])); | ||
| render(<App />); | ||
|
|
||
| await vi.waitFor(() => { | ||
| expect(screen.getByText('No products registered.')).toBeInTheDocument(); | ||
| }); | ||
| }); | ||
|
|
||
| it('redirects / to /products/:firstSlug when products exist', async () => { | ||
| mockListProducts.mockResolvedValue(ok(PRODUCTS)); | ||
| mockGetProductBySlug.mockResolvedValue(ok(PRODUCTS[0])); | ||
| mockListItemsForProduct.mockResolvedValue(ok([])); | ||
| render(<App />); | ||
|
|
||
| await vi.waitFor(() => { | ||
| expect(window.location.pathname).toBe('/products/helm'); | ||
| expect(screen.getByRole('heading', { name: /Helm/ })).toBeInTheDocument(); | ||
| }); | ||
| }); | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| }); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,117 @@ | ||
| import { render, screen } from '@testing-library/react'; | ||
| import { MemoryRouter, Route, Routes } from 'react-router-dom'; | ||
| import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; | ||
| import { ProductTabs } from './ProductTabs.js'; | ||
|
|
||
| // ── Mock api ────────────────────────────────────────────────────────────────── | ||
|
|
||
| const { mockListProducts } = vi.hoisted(() => ({ mockListProducts: vi.fn() })); | ||
|
|
||
| vi.mock('../lib/api.js', async (importOriginal) => { | ||
| const real = await importOriginal<typeof import('../lib/api.js')>(); | ||
| return { ...real, api: { ...real.api, listProducts: mockListProducts } }; | ||
| }); | ||
|
|
||
| // ── Helpers ─────────────────────────────────────────────────────────────────── | ||
|
|
||
| const PRODUCTS = [ | ||
| { product: { slug: 'helm', name: 'Helm' }, workflow: { stages_enabled: [] } }, | ||
| { | ||
| product: { slug: 'helm-playground', name: 'Helm Playground' }, | ||
| workflow: { stages_enabled: [] }, | ||
| }, | ||
| ]; | ||
|
|
||
| function ok<T>(data: T) { | ||
| return { ok: true as const, data }; | ||
| } | ||
| function err(message: string) { | ||
| return { ok: false as const, error: { type: 'network' as const, message } }; | ||
| } | ||
|
|
||
| function renderTabs(initialPath = '/products/helm') { | ||
| return render( | ||
| <MemoryRouter initialEntries={[initialPath]}> | ||
| <Routes> | ||
| <Route path="/products/:slug" element={<ProductTabs />} /> | ||
| <Route path="/products/:slug/items/:externalId" element={<ProductTabs />} /> | ||
| </Routes> | ||
| </MemoryRouter>, | ||
| ); | ||
| } | ||
|
|
||
| // ── Tests ───────────────────────────────────────────────────────────────────── | ||
|
|
||
| describe('ProductTabs', () => { | ||
| beforeEach(() => { | ||
| vi.useFakeTimers(); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| vi.useRealTimers(); | ||
| vi.restoreAllMocks(); | ||
| }); | ||
|
|
||
| it('renders a tab for each product', async () => { | ||
| mockListProducts.mockResolvedValue(ok(PRODUCTS)); | ||
| renderTabs(); | ||
|
|
||
| await vi.waitFor(() => { | ||
| expect(screen.getByText('Helm')).toBeInTheDocument(); | ||
| expect(screen.getByText('Helm Playground')).toBeInTheDocument(); | ||
| }); | ||
| }); | ||
|
|
||
| it('marks the active tab based on the current slug', async () => { | ||
| mockListProducts.mockResolvedValue(ok(PRODUCTS)); | ||
| renderTabs('/products/helm-playground'); | ||
|
|
||
| await vi.waitFor(() => { | ||
| const activeLink = screen.getByRole('link', { name: 'Helm Playground' }); | ||
| expect(activeLink).toHaveAttribute('aria-current', 'page'); | ||
| const inactiveLink = screen.getByRole('link', { name: 'Helm' }); | ||
| expect(inactiveLink).not.toHaveAttribute('aria-current'); | ||
| }); | ||
| }); | ||
|
|
||
| it('each tab links to /products/:slug', async () => { | ||
| mockListProducts.mockResolvedValue(ok(PRODUCTS)); | ||
| renderTabs(); | ||
|
|
||
| await vi.waitFor(() => { | ||
| expect(screen.getByRole('link', { name: 'Helm' })).toHaveAttribute('href', '/products/helm'); | ||
| expect(screen.getByRole('link', { name: 'Helm Playground' })).toHaveAttribute( | ||
| 'href', | ||
| '/products/helm-playground', | ||
| ); | ||
| }); | ||
| }); | ||
|
|
||
| it('shows error state when listProducts fails', async () => { | ||
| mockListProducts.mockResolvedValue(err('Network error')); | ||
| renderTabs(); | ||
|
|
||
| await vi.waitFor(() => { | ||
| expect(screen.getByText(/Failed to load products/)).toBeInTheDocument(); | ||
| }); | ||
| }); | ||
|
|
||
| it('renders nothing when product list is empty', async () => { | ||
| mockListProducts.mockResolvedValue(ok([])); | ||
| const { container } = renderTabs(); | ||
|
|
||
| await vi.waitFor(() => { | ||
| expect(container.firstChild).toBeNull(); | ||
| }); | ||
| }); | ||
|
|
||
| it('re-fetches products after polling interval', async () => { | ||
| mockListProducts.mockResolvedValue(ok(PRODUCTS)); | ||
| renderTabs(); | ||
|
|
||
| await vi.waitFor(() => expect(mockListProducts).toHaveBeenCalledTimes(1)); | ||
|
|
||
| await vi.runOnlyPendingTimersAsync(); | ||
| await vi.waitFor(() => expect(mockListProducts).toHaveBeenCalledTimes(2)); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| import { useCallback } from 'react'; | ||
| import { Link, useParams } from 'react-router-dom'; | ||
| import { api } from '../lib/api.js'; | ||
| import { usePolling } from '../hooks/usePolling.js'; | ||
|
|
||
| /** | ||
| * Horizontal product tabs rendered above every kanban/detail view. | ||
| * Polls every 5s so newly-registered products appear without a page reload. | ||
| */ | ||
| export function ProductTabs() { | ||
| const { slug } = useParams<{ slug?: string }>(); | ||
| const fetchProducts = useCallback(() => api.listProducts(), []); | ||
| const { data: products, error, loading } = usePolling(fetchProducts, 5_000); | ||
|
|
||
| // Blank bar while loading — avoids layout shift. | ||
| if (loading) { | ||
| return <div className="h-10 border-b border-gray-200 bg-white" />; | ||
| } | ||
|
|
||
| if (error) { | ||
| return ( | ||
| <div className="border-b border-gray-200 bg-white px-6 py-2"> | ||
| <p className="text-xs text-red-500">⚠ Failed to load products</p> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| if (!products?.length) return null; | ||
|
|
||
| return ( | ||
| <nav aria-label="Products" className="border-b border-gray-200 bg-white px-6"> | ||
| <div className="flex"> | ||
| {products.map((product) => { | ||
| const isActive = product.product.slug === slug; | ||
| return ( | ||
| <Link | ||
| key={product.product.slug} | ||
| to={`/products/${encodeURIComponent(product.product.slug)}`} | ||
| aria-current={isActive ? 'page' : undefined} | ||
| className={[ | ||
| 'px-4 py-3 text-sm font-medium transition-colors', | ||
| isActive | ||
| ? 'border-b-2 border-indigo-600 text-indigo-600' | ||
| : 'border-b-2 border-transparent text-gray-500 hover:text-gray-700', | ||
| ].join(' ')} | ||
| > | ||
| {product.product.name} | ||
| </Link> | ||
| ); | ||
| })} | ||
| </div> | ||
| </nav> | ||
| ); | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.