Skip to content

IGVF-3439 Display construct delivery methods on sample page#1039

Merged
forresttanaka merged 1 commit into
devfrom
IGVF-3439-construct-delivery
Apr 23, 2026
Merged

IGVF-3439 Display construct delivery methods on sample page#1039
forresttanaka merged 1 commit into
devfrom
IGVF-3439-construct-delivery

Conversation

@forresttanaka
Copy link
Copy Markdown
Collaborator

Code Review — Branch IGVF-3439-construct-delivery

Reviewer: Claude Sonnet 4.6
Date: 2026-04-22
Comparing: dev...IGVF-3439-construct-delivery


Summary

This branch renames and expands the "nucleic acid delivery" sample field to the more general "construct delivery methods" field. It also applies a minor scroll-margin Tailwind class fix on the schema property page. A routine Tailwind CSS patch-version bump (4.2.2 → 4.2.4) is included in package-lock.json.


Changed Files

File Change Type
components/common-data-items.js Renamed/updated field display logic
pages/profiles/[...profile].js Tailwind scroll-margin class tweak
package-lock.json Tailwind CSS 4.2.2 → 4.2.4 (patch bump)

Detailed Review

components/common-data-items.js

What changed:
The nucleic_acid_delivery string field (rendered as a single value) is replaced by construct_delivery_methods, which is an array of strings. The new array is sorted case-insensitively and joined with ", " for display.

Observations:

  • Array guard is correct. The condition item.construct_delivery_methods?.length > 0 properly handles undefined, null, and empty arrays before rendering.
  • Sorting is sensible. Using _.sortBy with .toLowerCase() gives a stable, locale-agnostic alphabetical sort. Using the existing Lodash import is consistent with the rest of the file.
  • commonProperties updated correctly. construct_delivery_methods was added and nucleic_acid_delivery removed from SampleDataItems.commonProperties, keeping the property list in sync with the rendered output. The list remains alphabetically ordered.
  • No PropTypes change needed. item is typed as PropTypes.object, so no update is required there.
  • Potential minor nit: Joining with ", " is simple and readable, but if construct_delivery_methods values can themselves contain commas (unlikely for enum-style values, but worth confirming against the schema), a different separator might be safer. This is low risk for controlled vocab fields.

pages/profiles/[...profile].js

What changed:
The Tailwind responsive scroll-margin class was changed from @2xl:scroll-mt-32 to @2xl:scroll-mt-44.

Observations:

  • This is a layout/visual correction. The change increases the scroll margin at the @2xl container breakpoint from 8rem to 11rem, presumably to prevent a sticky header from obscuring highlighted schema properties when scrolled into view.
  • The fix is surgical and correct in isolation. No logic is affected.

package-lock.json

  • Tailwind CSS patch bump (4.2.2 → 4.2.4). This is a routine dependency update with no code impact. No new packages added.

Issues Found

None. The changes are small, focused, and internally consistent.


Recommendation

Merge approved.

The changes are minimal, well-scoped, and correct:

  • The field rename properly handles the transition from a scalar string to an array, with appropriate null-safety and sorting.
  • The commonProperties list is kept in sync.
  • The scroll-margin fix is a straightforward visual correction.
  • No security concerns, no logic regressions, no missing test surface beyond what the existing pattern covers.

The only thing worth a second look (not a blocker) is confirming with the data model that values in construct_delivery_methods are controlled-vocabulary strings that will never themselves contain commas, to validate the join separator choice.

* Update npm packages. When highlighting a property on an individual schema page, have it scroll farther down to avoid it getting hidden by the sticky header.
* Change display of `nucleic_acid_delivery` to sorted display of `construct_delivery_methods`.
* Change `SampleDataItems.commonProperties` to use `construct_delivery_methods` instead of `nucleic_acid_delivery`.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the Sample page to display the new construct_delivery_methods field (replacing the older nucleic-acid-specific field), adjusts schema-property scroll offset styling, and updates the lockfile for newer Tailwind-related packages.

Changes:

  • Replace nucleic_acid_delivery display with sorted/joined construct_delivery_methods on sample-derived object pages.
  • Adjust schema property anchor scroll margin at the @2xl container breakpoint.
  • Update package-lock.json with Tailwind CSS patch updates (and additional transitive/tooling bumps).

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.

File Description
components/common-data-items.js Renders construct_delivery_methods (array) in SampleDataItems and updates commonProperties.
pages/profiles/[...profile].js Tweaks @2xl scroll-margin class for schema property anchors.
package-lock.json Locks updated Tailwind packages and other related dependency versions/metadata.

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

Comment thread pages/profiles/[...profile].js
@forresttanaka forresttanaka merged commit 05bf7e8 into dev Apr 23, 2026
10 checks passed
@forresttanaka forresttanaka deleted the IGVF-3439-construct-delivery branch April 23, 2026 22:47
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