feat: build modular element detail page #64

ronjalh · 2026-10-16T15:42:06Z

Summary

Build a modular, route-backed element detail page using the existing variant and difficulty components.

What's in it

Add /elements/:elementId with loading, not-found, and error/retry states.
Add focused header, metadata, diagram, and action sections.
Load element, apparatus, group, image, and variant data through a local catalog adapter.
Reuse ElementVariants and DifficultyBadge; use existing CSS variables for page and badge colors.
Add catalog and variant rendering tests, plus the required AI-usage entry.

Choices

The GraphQL server is not implemented yet, so the page reads the checked-in catalog JSON behind a separate adapter that can be replaced with an API client later. No new color values are introduced.

src/lib/catalog.ts reads the catalog from data/*.json in the frontend, so the whole catalog goes into the bundle (JS grows from about 224 kB to 496 kB). This is a stand-in until the GraphQL element / catalogItem queries are used.
/ redirects to a fixed element (UB-1.101-a) while this is the only page.
The drawings in data/images are copied to dist/data/images by vite.config.ts on build, so they also work after npm run build.

Testing

npm run lint passed.
npm run build passed.
npm test -- --run: 26 passed, 23 skipped.
Focused ElementVariants tests passed after the final badge change.

Not done (still open)

The action buttons provide accessible feedback but are not connected to routine navigation or sign-in; see Create "add to gymnast"-button component #47.
Replace the local catalog adapter with GraphQL when the server is available.
Add page-level tests for loading, error, and not-found states.

Dependencies

This draft is stacked on PR #62, which in turn depends on PR #61. Its temporary base is 48-element-variant-card; retarget it to dev after #62 merges.

Part of #63

Generated with GitHub Copilot

anettkva · 2026-10-16T19:23:34Z

Checked against 48-element-variant-card: it merges cleanly, and lint, type check, build and all 49 tests pass. Two things to fix before merging:

Prettier fails on 11 of the changed files. The new files use double quotes and semicolons, while the repo uses single quotes and no semicolons. Some also lack the final newline. Run npx prettier --write on the changed files.
The drawings are missing in the build. catalog.ts uses /data/images/.... That works in npm run dev, but data/ is not in public/, so the images are not copied into dist/ and all image links break after npm run build.

Fine for now, but please mention it in the PR description:

catalog.ts reads the catalog from data/*.json in the frontend. The whole catalog goes into the bundle (JS grows from about 224 kB to 496 kB). That's OK as a stand-in, but it should be replaced by the GraphQL element / catalogItem queries when the API is ready.
/ redirects to a fixed element (UB-1.101-a). That's OK while this is the only page.

Generated with claude code

… the badge - Merge 48-element-variant-card (keeps this branch's badge and variant list). - Prettier on all changed source files (single quotes, no semicolons). - vite.config.ts copies data/images to dist/data/images on build, so the drawings also work after npm run build. - DifficultyBadge: title and a visually hidden label instead of role=img.

xeniag · 2026-10-18T15:15:38Z

src/components/elements/ElementActions.tsx

Great work setting up the aria-live region for screen readers!

Two quick fixes before merging:

Visual Feedback: p.sr-only hides the status text from sight, leaving sighted users with no feedback when clicking the buttons. Render the status message visually (e.g. <p role="status" className="text-xs text-muted-foreground">) so everyone sees why the feature is locked/unbuilt.

Icon Sizing: Add an explicit size class like size-4 to <Plus /> and <LockKeyhole /> to prevent SVG scaling layout glitches across different browsers.

xeniag · 2026-10-18T15:19:09Z

src/components/elements/ElementDetailHeader.tsx

Looking good! Two quick accessibility improvements before merging:

Back Button aria-label: Add aria-label="Go back to previous page" to the back button so screen readers give a descriptive context when navigating by interactive elements.

Conditional Description Guard: Wrap <p> with {element.description && ...} so empty description fields don't leave empty <p> elements in the DOM.

xeniag · 2026-10-18T15:20:12Z

src/components/elements/ElementDetailPage.tsx

ElementDetailPage.tsx state machine and data-fetching flow look very solid!

One quick styling polish: Add heading font utility classes (text-3xl font-bold) to the <h1> elements in the not-found and error conditional branches so they don't render as unstyled browser defaults when a 404 or network error occurs.

xeniag · 2026-10-18T15:22:21Z

src/components/elements/ElementDiagram.tsx

Just a tiny polish suggestion when you get a chance:

Alt Text: We could simplify the alt attribute to Illustration for ${element.code}${element.name ? : ${element.name} : ''}. Screen readers automatically announce "Image", so keeping it focused on the code/name avoids reading out long description text or repeating words like "sequence".

Semantics: Since there isn't a visible <figcaption>, changing <figure> to a standard <div> (or adding a <figcaption> if we want a caption below the image) will keep the HTML structure super tight.

xeniag · 2026-10-18T15:23:35Z

src/components/elements/ElementMetadataCard.tsx

Love how clean and well-structured ElementMetadataCard.tsx is coming along! Using <dl>, <dt>, and <dd> inside a semantic <section> gives it fantastic document structure for accessibility.

Just a couple of small suggestions to consider:

Badge Label Consistency: Inside MetadataItem, element.isVault ? 'D-value' : 'Difficulty' works great, but for non-vaults with an unmapped difficulty letter, ElementValueBadge receives label="Difficulty value". We might want to pass label="Difficulty" there too so screen reader announcements stay consistent regardless of whether it falls back to a numeric value or renders DifficultyBadge.

Card Padding Utilities: If you're using Tailwind v4 variable utilities elsewhere in the layout, swapping style={{ backgroundColor: 'var(--background-color-element)' }} for bg-(--background-color-element) will keep the styling approach uniform across your UI components.

Really nice work on the layout and semantic markup!

xeniag · 2026-10-18T15:34:07Z

src/components/ui/ElementVariants.tsx

Looks good! Just two quick accessibility tweaks to consider:

Replace inline style with Tailwind v4 syntax: Swap style={{ backgroundColor: 'var(--background-color-element)' }} for className="w-full bg-(--background-color-element)" on <Card> to keep your utility class pattern consistent across components.

Standardize ElementValueBadge label: Inside ElementValueBadge, pass label={variant.isVault ? 'D-value' : 'Difficulty'} (matching ElementMetadataCard) so screen reader announcements stay completely uniform across fallback states.

xeniag · 2026-10-18T15:35:26Z

src/lib/catalog.ts

Looks great! Just two small data-handling improvements to consider:

Fallback for Missing Image Filename: If record.image is undefined or null on a record, ${import.meta.env.BASE_URL}data/images/${apparatusId}/${record.image} will produce an invalid image path ending in /undefined. Setting imageUrl to record.image ? ... : null ensures the component cleanly triggers its "No illustration available" fallback state.

Dynamic Base URL Trailing Slashes: Depending on how import.meta.env.BASE_URL is configured in Vite, concatenating without normalization can lead to double slashes (e.g. //data/images/...). Replacing import.meta.env.BASE_URL with a light path-join or stripping extra slashes will prevent unexpected 404s in production subfolder deployments.

xeniag · 2026-10-18T15:36:13Z

src/lib/catalog.test.ts

Looks good! Just two quick test suite improvements to consider:

Explicit imageUrl test for missing images: Add a test case for an element record where image is undefined or null to ensure getElementDetail sets imageUrl to null instead of building an invalid path ending in /undefined.

Loosen strict path structure in imageUrl match: If import.meta.env.BASE_URL changes between environments (e.g., local preview vs. CI/staging subpaths), checking expect(detail?.imageUrl).toContain('UB-1.101-a.webp') keeps the assertion clean while preventing fragile regex matches on leading slash patterns.

xeniag · 2026-10-18T15:39:43Z

src/lib/difficulty.ts

Looks good! Just two quick improvements to consider:

Floating-point comparison safety in getDifficultyLetter: Rather than relying on exact equality after toFixed(1), using an epsilon check (e.g. Math.abs(difficultyValue - value) < 0.05) prevents precision edge cases if dynamic values like 0.10000000000000002 are passed in.

Reverse lookup map: Pre-building a Map<number, DifficultyLetter> once at module scope avoids running Object.entries() array iterations on every lookup call.

xeniag · 2026-10-18T15:41:49Z

src/main.tsx

Looks good! Just two quick structural improvements to consider:

Move CSS imports into App.tsx or a single styles entry point: Keeping global styles (variables.css, global.css, index.css) organized in App.tsx or bundled through a central stylesheet keeps main.tsx cleanly focused on bootstrapping the React root and router.

Set basename on BrowserRouter for subpath deployments: If hosting on a subfolder or virtual machine path (like Vite's import.meta.env.BASE_URL), passing <BrowserRouter basename={import.meta.env.BASE_URL}> prevents routing 404s on page refresh or direct navigation.

Ronja added 4 commits October 16, 2026 17:36

feat: add catalog lookup for element details

fb142ce

fix: reuse difficulty badge in element variants

3c1c370

feat: build modular element detail page

9f23af7

fix: use existing light text token for actions

419666f

ronjalh self-assigned this Oct 16, 2026

ronjalh force-pushed the 48-element-variant-card branch from 9f23af7 to 41097f6 Compare October 16, 2026 15:47

ronjalh marked this pull request as ready for review October 16, 2026 16:01

ronjalh requested a review from a team as a code owner October 16, 2026 16:01

ronjalh mentioned this pull request Oct 17, 2026

feat: set up Apollo Client in the frontend #75

Open

5 tasks

xeniag reviewed Oct 18, 2026

View reviewed changes

Uh oh!

feat: build modular element detail page #64

feat: build modular element detail page #64

ronjalh commented Oct 16, 2026 •

edited

Loading

Uh oh!

anettkva commented Oct 16, 2026

Uh oh!

xeniag Oct 18, 2026

Uh oh!

xeniag Oct 18, 2026

Uh oh!

xeniag Oct 18, 2026

Uh oh!

xeniag Oct 18, 2026

Uh oh!

xeniag Oct 18, 2026

Uh oh!

xeniag Oct 18, 2026

Uh oh!

xeniag Oct 18, 2026

Uh oh!

xeniag Oct 18, 2026

Uh oh!

xeniag Oct 18, 2026

Uh oh!

xeniag Oct 18, 2026

Uh oh!

Uh oh!

feat: build modular element detail page #64

Are you sure you want to change the base?

feat: build modular element detail page #64

Conversation

ronjalh commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What's in it

Choices

Testing

Not done (still open)

Dependencies

Uh oh!

anettkva commented Oct 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ronjalh commented Oct 16, 2026 •

edited

Loading