Skip to content

feat(resources): render annotations on ResourceListItem#1249

Merged
cliffhall merged 2 commits into
v2/mainfrom
v2/issue-1238-resourcelistitem-annotations
Apr 27, 2026
Merged

feat(resources): render annotations on ResourceListItem#1249
cliffhall merged 2 commits into
v2/mainfrom
v2/issue-1238-resourcelistitem-annotations

Conversation

@cliffhall
Copy link
Copy Markdown
Member

Closes #1238.

Summary

  • ResourceListItem now renders audience and priority annotations via AnnotationBadge, matching the pattern already used in ResourcePreviewPanel. Closes the second half of the Phase 5 spec target (the title ?? name half had already landed).
  • Layout: a wrapping Group keeps the title and badges on one line when the row is wide and lets badges drop below the title when the container narrows. When the resource has no annotations (or only empty/undefined fields), the row renders the bare Text exactly as before — no extra elements, no leaked spacing.
  • Adds stories for: audience-only, priority-only, both audience and priority, and a ResourceTemplate with annotations.

Test plan

  • npm run validate (format:check, lint, build) passes.
  • npx vitest run --project=storybook src/components/groups/ResourceListItem — 8/8 stories pass.
  • Spot-check ResourcesScreen.stories.tsx > WithResources in dev — the sidebar list should now show badges on config.json (audience + priority) and package.json (audience), with unannotated rows unchanged.

🤖 Generated with Claude Code

Half-finished Phase 5 spec target: ResourceListItem already used
title ?? name but ignored resource.annotations entirely. Now renders
audience and priority via the existing AnnotationBadge element, matching
how ResourcePreviewPanel surfaces the same fields.

Layout: a wrapping Group keeps the title + badges on a single line when
the row is wide, and lets badges drop below when narrow. When neither
annotation is present, the row renders the bare Text (no Group, no
leaked spacing).

Adds stories for audience-only, priority-only, both, and a
ResourceTemplate with annotations.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@cliffhall
Copy link
Copy Markdown
Member Author

@claude review

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 27, 2026

Claude finished @cliffhall's task in 2m 40s —— View job


Code Review

  • Gather context and read changed files
  • Review ResourceListItem.tsx changes
  • Review ResourceListItem.stories.tsx changes
  • Check adherence to project coding standards
  • Post final review

Overall this is a clean, focused change that correctly follows the existing ResourcePreviewPanel pattern. One type-safety issue worth addressing before merge.

TypeScript Narrowing Issue

clients/web/src/components/groups/ResourceListItem/ResourceListItem.tsx, lines 33–34 and 49–53

With strict: true in tsconfig.app.json, storing checks in boolean variables does not narrow the types of the original values:

const hasAudience = audience !== undefined && audience.length > 0;  // boolean
// ...
{hasAudience && (
  <AnnotationBadge facet="audience" value={audience} />  // audience is still Role[] | undefined here
)}

audience remains Role[] | undefined inside the hasAudience && branch because TypeScript can't infer that a boolean variable implies anything about another variable's type. AnnotationBadgeProps.value is Role[] | number | boolean — no undefined — so this is a potential type error.

Compare with how ResourcePreviewPanel handles it, where TypeScript narrows correctly through an inline expression:

{annotations?.audience && (
  <AnnotationBadge facet="audience" value={annotations.audience} />
)}

The simplest fix, matching ResourcePreviewPanel's pattern:

{audience !== undefined && audience.length > 0 && (
  <AnnotationBadge facet="audience" value={audience} />
)}
{priority !== undefined && (
  <AnnotationBadge facet="priority" value={priority} />
)}

This lets TypeScript narrow audience to Role[] and priority to number inside the right-hand side. The hasPriority/hasAudience variables become unused and can be removed, simplifying the component. Fix this →

What's Well Done

  • Subcomponent constants (RowGroup, BadgeGroup) correctly use Group.withProps() following the project conventions.
  • Responsive layout: RowGroup with justify: "space-between" and wrap: "wrap" correctly handles narrow containers without any inline styles.
  • No-annotation regression protection: the else branch renders the bare <Text> exactly as before — no extra wrapper elements or spacing when annotations are absent.
  • Defensive empty-array check: audience.length > 0 is more precise than ResourcePreviewPanel's truthy check (which would render an empty badge for audience: []). Worth keeping, and worth aligning ResourcePreviewPanel eventually.
  • Story coverage: all four annotation combinations (audience-only, priority-only, both, template) are well covered.

Minor Nit

WithAudience and WithAudienceAndPriority both use name: "config.json" / uri: "file:///config.json". Using a distinct resource name per story (e.g. "settings.json" for audience-only) makes the Storybook canvas slightly clearer at a glance, but this is purely cosmetic.

- Inline the audience/priority narrowing checks instead of routing
  through aliased booleans, matching ResourcePreviewPanel's pattern
  and avoiding reliance on TS aliased-condition narrowing for the
  inner badges.
- Use a distinct resource name (settings.json) for WithAudience so
  the Storybook canvas reads more clearly across the four annotation
  stories.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@cliffhall
Copy link
Copy Markdown
Member Author

Thanks for the review. Pushed 3cc726a0 addressing both points.

TypeScript narrowing — Applied. Aliased booleans removed; the audience and priority checks are now inline in the JSX, matching ResourcePreviewPanel's pattern. Kept audience.length > 0 rather than the truthy-only check (an empty array is truthy in JS, and we don't want a stray empty badge if a server reports audience: []). One outer showBadges boolean remains for the wrapper conditional, but it doesn't need to narrow anything — it just decides whether to render the RowGroup.

Worth noting: the original code did compile under strict: true because TS 4.4+ supports aliased-condition narrowing for simple discriminants. But the inline form is sturdier (it doesn't rely on that subtlety) and matches the established pattern, so it's the right call.

Cosmetic nit — Applied. WithAudience now uses settings.json so the four annotation stories read distinctly in the canvas (config.json → both, settings.json → audience only, schema.sql → priority only, User Profile → template).

Validate and the 8 ResourceListItem stories still pass.

@cliffhall
Copy link
Copy Markdown
Member Author

@claude review

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 27, 2026

Claude finished @cliffhall's task in 1m 18s —— View job


Code Review

  • Gather context
  • Review ResourceListItem.tsx changes
  • Review ResourceListItem.stories.tsx changes
  • Check adherence to project coding standards
  • Post final review

Both points from the previous review have been cleanly addressed. The PR is in good shape.

TypeScript Narrowing — Resolved ✓

The aliased boolean variables (hasAudience, hasPriority) are gone. The JSX now uses inline narrowing:

{audience !== undefined && audience.length > 0 && (
  <AnnotationBadge facet="audience" value={audience} />
)}
{priority !== undefined && (
  <AnnotationBadge facet="priority" value={priority} />
)}

The one remaining boolean showBadges (line 32–33) is purely structural — it gates whether to render the RowGroup wrapper vs. the bare label. TypeScript doesn't need to narrow through it; both audience and priority are re-narrowed inline inside BadgeGroup. The logic is consistent: showBadges is the logical OR of the two same inline conditions, so it's impossible for showBadges to be true while both inner conditions are false. No stray empty BadgeGroup can be rendered.

The label const (line 34) to avoid duplicating <Text fw={500}> in both branches is a nice touch.

Cosmetic Nit — Resolved ✓

All four new stories use distinct resource names, making the Storybook canvas unambiguous:

Story Resource
WithAudience settings.json
WithPriority schema.sql
WithAudienceAndPriority config.json (with title)
TemplateWithAnnotations User Profile (template)

Overall Assessment

The implementation is clean and follows all project conventions:

  • RowGroup and BadgeGroup correctly use Group.withProps() with 2+ props each.
  • No inline styles, no raw hex colors, no div for layout.
  • The audience.length > 0 guard is stricter than ResourcePreviewPanel's truthy check — an intentional improvement worth keeping and eventually back-porting there.
  • The no-annotation code path renders the bare <Text> exactly as before — no extra wrapper elements or spacing leak.
  • Story coverage is comprehensive: all four annotation combinations are represented.

LGTM. Ready to merge.

@cliffhall cliffhall merged commit f1d6fbd into v2/main Apr 27, 2026
2 checks passed
@cliffhall cliffhall deleted the v2/issue-1238-resourcelistitem-annotations branch April 27, 2026 21:23
@cliffhall cliffhall added the v2 Issues and PRs for v2 label Apr 28, 2026
cliffhall added a commit that referenced this pull request Apr 28, 2026
* docs(agents): require version label on new issues and PRs (#1256)

Recent v2 PRs (#1247, #1249, #1250, #1252, #1255) all landed without a
v2 label, making version-filtered queries unreliable. Codify the
expectation directly in the issue-driven workflow: each branch / board
gets a fixed label, and the label is set at create time via the
gh create flag rather than backfilled.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Also fix stale "PR against main" line

The PR-target line still said "Open a PR against main", which is
correct only for v1 work. Mirror the version-label mapping above so
the PR base matches the board.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@cliffhall cliffhall self-assigned this May 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v2 Issues and PRs for v2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant