Conversation
✅ Deploy Preview for cell-catalog ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where buttons without valid links were being displayed to users. When certificate links (Certificate of Analysis or hPSCreg Certificate) are empty, the buttons should not be rendered at all rather than showing non-functional buttons.
Changes:
- Modified
getDefaultButtonhelper function to return null when href is empty/falsy - Converted the arrow function from implicit return (using parentheses) to explicit return (using braces) to add the conditional check
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const getDefaultButton = (label: string, href: string) => { | ||
| if (!href) { | ||
| return null; | ||
| } | ||
| return ( | ||
| <DefaultButton | ||
| key={href} | ||
| href={href} |
There was a problem hiding this comment.
getDefaultButton now explicitly handles missing/empty href, but its signature still requires href: string. To align the typing with the intended behavior (and allow callers to pass through nullable/optional data without type assertions), consider changing the parameter type to href?: string (or string | null | undefined) and, if frontmatter can contain whitespace-only values, treating href.trim() as empty too.
| const getDefaultButton = (label: string, href: string) => { | |
| if (!href) { | |
| return null; | |
| } | |
| return ( | |
| <DefaultButton | |
| key={href} | |
| href={href} | |
| const getDefaultButton = (label: string, href?: string | null) => { | |
| const normalizedHref = href && href.trim(); | |
| if (!normalizedHref) { | |
| return null; | |
| } | |
| return ( | |
| <DefaultButton | |
| key={normalizedHref} | |
| href={normalizedHref} |
rugeli
left a comment
rugeli
left a comment
There was a problem hiding this comment.
checked both disease and original lines, particularly 122 and 120 (the ones without hPSCreg links), and the buttons are displaying/hidden as expected!
While reviewing, I noticed one field name inconsistency between md files and config (eu_hpsc_reg vs hPSCreg_certificate_link), I'll fix it separately.
3 participants
Problem
closes #244
Solution
Check that an href exists
Type of change
Please delete options that are not relevant.
Steps to Verify: