IGVF-2753 Add downstream analysis files to file-set file download options

[forresttanaka]

No description provided.

[forresttanaka]

Code Review — IGVF-2753-recursive-download

Reviewed by GitHub Copilot (Claude Sonnet 4.6)

Files Changed

Status	File
Added	components/__tests__/radio-card-group.test.tsx
Added	components/radio-card-group.tsx
Modified	components/batch-download-fileset.tsx
Modified	components/search/__tests__/list-renderer.test.js
Modified	components/search/list-renderer/index.tsx
Renamed + Modified	components/search/list-renderer/search-list-item.js → .tsx
Modified	globals.d.ts
Modified	lib/batch-download/fileset-controller.ts
Modified	pages/measurement-sets/[id].js
Modified	styles/globals.css

---

Summary

This PR adds a recursive/downstream file-download option to the measurement-set page. It introduces a new <RadioCardGroup> component used to present the download choices in a modal, extends FileSetController to optionally append ?include_downstream=true to the download URL, and migrates search-list-item.js from PropTypes to TypeScript.

---

components/radio-card-group.tsx — New Component

The component is well-structured. Notable highlights:

• Accessibility is correct. The group uses <fieldset> + <legend>, each card is a <label htmlFor> paired with a <input type="radio">, and hideLegend uses sr-only rather than display:none, so screen readers still have the context.
• Context guard. Card throws an explicit error if used outside a RadioCardGroup, which is good developer ergonomics.
• Unique input IDs. Prefixing with name (${name}-${id}) means multiple <RadioCardGroup> instances on the same page won't collide.
• Container queries. The responsive grid uses @lg and @4xl breakpoints with @container, which is correct for a component that may be embedded inside a modal at an unpredictable viewport width.
• twMerge usage. The label class string mixes a template literal with twMerge. This is fine, but note that Tailwind's IntelliSense and purging won't statically detect cursor-default and cursor-pointer when they're inside a ternary inside a template literal inside twMerge. In practice this is unlikely to cause a problem because Tailwind v4 scans class strings more broadly, but it's worth being aware of.

No issues found.

---

components/batch-download-fileset.tsx — Modified

• Three download options are correctly modelled by the DOWNLOAD_OPTIONS const-object and the DownloadOption derived type.
• State reset on navigation. The useEffect that resets selectedOption and isOptionDisabled when fileSet["@id"] changes is the right pattern for a component that may be reused across client-side navigations.
• useMemo dependency array. fileSet["@id"] is listed alongside selectedOption. The comment explains why — good.
• Early return null. The guard if (!controller.offerDownload) return null is explicit and commented, and correctly avoids rendering the actuator when there are no files.

No issues found.

---

lib/batch-download/fileset-controller.ts — Modified

• includeDownstream: boolean is correctly typed.
• URL construction is straightforward. The ?include_downstream=true query parameter is only appended when the flag is true, which is the correct behaviour. The empty-string branch produces no trailing characters.

No issues found.

---

components/search/list-renderer/search-list-item.js → .tsx

Straightforward JS-to-TS migration:

• All PropTypes declarations replaced with inline TypeScript prop types.
• All children-only props typed as { children: ReactNode }.
• Item props typed as { item: DatabaseObject }.
• Children.count() fix. The original code used children.length, which doesn't work correctly for a ReactNode (it's not an array). Children.count() is the correct React API.

No issues found.

---

components/search/list-renderer/index.tsx — Modified

• Fallback's item prop changed from Record<string, any> to DatabaseObject. This is a straightforward type improvement with no behavioural change.

No issues found.

---

globals.d.ts — Modified

Three optional properties added to DatabaseObject: description, name, and summary. These are common enough across object types to belong on the base interface.

No issues found.

---

pages/measurement-sets/[id].js — Modified

<BatchDownloadFileSet fileSet={measurementSet} /> is added to the <ObjectPageHeader> in the correct position (before other header controls). The import is grouped with the other component imports.

No issues found.

---

styles/globals.css — Modified

Three CSS custom properties added for radio-card colours in both the light and dark-mode blocks:

--color-radio-card-outline
--color-radio-card-outline-checked
--color-radio-card-button

All three are defined in both blocks. Light mode uses gray-300 for the outline and blue-500 for the checked state and button accent; dark mode uses gray-700 for the outline (appropriate contrast) and the same blue-500 for checked/accent.

No issues found.

---

components/search/__tests__/list-renderer.test.js — Modified

The PseudobulkSet tests were previously nested inside a describe block belonging to IndexFile. They have been moved out to their own top-level describe, which is the correct organisation. The test body indentation and assertion style has been normalised to match the surrounding file. A new assertion for getAccessoryDataPaths is also added to the workflows test.

No issues found.

---

components/__tests__/radio-card-group.test.tsx — New

Four test cases covering:

1. Rendering a group with a visible legend, correct card labels/descriptions, checked state, and click callback.
2. Rendering with a hidden legend (sr-only).
3. Rendering a disabled card.
4. Throwing when <Card> is used outside of <RadioCardGroup> (uses the named Card export to ensure the declaration itself is covered, not just RadioCardGroup.Card).

Coverage is thorough for the component's API surface.

No issues found.

---

Overall Assessment

The code is well-written, consistent with the existing codebase conventions, and passes TypeScript checking with no errors. Accessibility, dark-mode support, and state management are all handled correctly. Approve.

[forresttanaka]

Code Review: IGVF-2753-recursive-download

• Repository: IGVF-DACC/igvf-ui
• Base branch: dev
• Review branch: IGVF-2753-recursive-download
• Model: GPT-5.4
• Recommendation: Recommend merge

Summary

I do not have any actionable code-review findings for the current branch.

The batch-download changes are internally consistent across the UI, controller selection, and measurement-set page integration:

• BatchDownloadFileSet now offers three download modes and maps them to the expected controller behavior.
• FileSetController appends ?include_downstream=true only for the downstream-analysis option.
• The measurement-set page now exposes the same batch-download entry point as other file-set pages.

Findings

No blocking bugs, regressions, or missing integration changes were identified in the reviewed diff.

Residual Risk

There is still some verification risk because the new FileSetController URL contract is not covered by automated tests in this branch, but that omission is intentional per discussion and is not being treated as a review finding.

I also could not fully validate with the workspace typecheck task because the configured fallback command failed in this environment with tsc: command not found.

Merge Recommendation

Based on the code reviewed, I recommend merging this branch.

[Copilot]

Pull request overview

Adds a new file-set batch download option that can include downstream analysis files, and updates the UI/typing around search list rendering.

Changes:

• Add a 3rd batch-download option that appends ?include_downstream=true to the @@all-files endpoint.
• Introduce a reusable <RadioCardGroup> UI component and use it for batch-download option selection.
• Tighten typings in search list item components and adjust tests accordingly.

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
styles/globals.css	Adds CSS variables intended for radio-card styling (light/dark).
pages/measurement-sets/[id].js	Adds the batch download actuator to measurement set headers.
lib/batch-download/fileset-controller.ts	Adds includeDownstream flag and conditionally appends include_downstream=true to download URL.
globals.d.ts	Expands DatabaseObject with optional name, description, summary.
components/search/list-renderer/search-list-item.tsx	Removes PropTypes usage and adds TS typings; fixes React children spacing logic.
components/search/list-renderer/index.tsx	Narrows Fallback renderer item type to DatabaseObject.
components/search/tests/list-renderer.test.js	Restructures/extends renderer tests (notably PseudobulkSet).
components/radio-card-group.tsx	New radio-card group + card implementation via context.
components/batch-download-fileset.tsx	Adds new download options and switches option UI to <RadioCardGroup>.
components/tests/radio-card-group.test.tsx	New unit tests for <RadioCardGroup> behavior.

---

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