Pulling fix/empty-gene-link into develop#1002
Conversation
The GenesDisplay component is simplified by removing the conditional rendering of an empty fragment when the genes array is empty. The component now always renders a fragment containing the mapped LinkTag components. This change reduces the complexity of the component and improves readability.
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
…e a name The StrainDetailsCard component now checks if the genes exist and have a name before displaying them. This prevents the component from rendering empty or undefined gene names, improving the user experience.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1002 +/- ##
========================================
Coverage 90.16% 90.16%
========================================
Files 316 316
Lines 18893 18894 +1
Branches 800 799 -1
========================================
+ Hits 17035 17036 +1
Misses 1855 1855
Partials 3 3
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent rendering “empty” gene links in the strain details UI by filtering out genes with empty name values before displaying them.
Changes:
- Filter
genesinStrainDetailsCardto exclude entries with emptyname, and renderGenesDisplayonly for non-empty results. - Simplify
GenesDisplayto always render the mappedLinkTaglist (removing the explicit empty-array early return).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
packages/ui-dsc/src/catalog/StrainDetailsCard.tsx |
Filters out empty gene names and avoids rendering GenesDisplay for an empty post-filter list. |
packages/ui-dsc/src/catalog/GenesDisplay.tsx |
Simplifies rendering to a direct map over genes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| content: pipe( | ||
| genes, | ||
| OfromNullable, | ||
| Omap((g) => <GenesDisplay genes={g} />), | ||
| OgetOrElse(() => <></>), | ||
| OgetOrElse( | ||
| () => [] as NonNullable<NonNullable<StrainQuery["strain"]>["genes"]>, | ||
| ), | ||
| Afilter(({ name }) => !isEmpty(name)), | ||
| Amatch( | ||
| () => <></>, | ||
| (g) => <GenesDisplay genes={g} />, | ||
| ), |
There was a problem hiding this comment.
The new behavior that filters out genes with empty name values changes what is rendered for the "Associated Gene(s)" row. There is existing test coverage for StrainDetailsCard, but it doesn't appear to cover this rendering logic; adding a test case where data.genes contains an empty-name gene would help prevent regressions (and ensures the empty link bug stays fixed).
| {genes.map((gene) => ( | ||
| <LinkTag key={gene.name} item={gene.name} route="gene" /> | ||
| ))} |
There was a problem hiding this comment.
GenesDisplay will render a LinkTag even when gene.name is an empty string, which produces an empty/invalid link (e.g. href="/gene/" and title="View gene "). Since GenesDisplay is used in multiple places (not all of which filter names), consider filtering out genes with empty names inside GenesDisplay (or inside LinkTag) so this is handled consistently across the app.
| {genes.map((gene) => ( | |
| <LinkTag key={gene.name} item={gene.name} route="gene" /> | |
| ))} | |
| {genes | |
| .filter((gene) => gene.name && gene.name.trim() !== "") | |
| .map((gene) => ( | |
| <LinkTag key={gene.name} item={gene.name} route="gene" /> | |
| ))} |
No description provided.