Skip to content

Refactor Digital Specimen overview: Data cards#247

Merged
MelanieTheLion merged 14 commits into
mainfrom
feature/refactor-ds-overview
May 8, 2026
Merged

Refactor Digital Specimen overview: Data cards#247
MelanieTheLion merged 14 commits into
mainfrom
feature/refactor-ds-overview

Conversation

@MelanieTheLion
Copy link
Copy Markdown
Collaborator

@MelanieTheLion MelanieTheLion commented Apr 30, 2026

In this PR:

  • Created first set up with the Digital Specimen cards for the overview page with their respective data
  • Cards will not be shown if there is no data for that specific subject
  • The hero with the specific info on the digital specimen

Not in this PR:

  • testing
  • Image card

Important: This page is not yet visible and routed to. But due to its size, we will merge parts of the page.

What it looks like:
image

Comment thread src/utils/DataMappers/schemas/DigitalSpecimenSchema.ts
@MelanieTheLion MelanieTheLion self-assigned this Apr 30, 2026
@MelanieTheLion MelanieTheLion marked this pull request as ready for review April 30, 2026 07:50
.ds-card-citation {
max-height: var(--citation-content-spacing);
width: 100%;
background-color: #F2F3F8;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Does this need a css var?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes!! Changed!

Comment thread src/components/Hero/Hero.scss Outdated
justify-content: space-between;
margin-block-end: var(--spacing-l);

div {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

using html tags, especially div could lead to hard to find css bugs. Is this css specificity needed? Can't you use a class or attribute selector?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, changed!!

Comment thread src/components/Hero/Hero.tsx Outdated
<div id="hero-title">
<h1>{title}</h1>
{isHtml ? (
<h1 dangerouslySetInnerHTML={{ __html: title }} />
Copy link
Copy Markdown
Collaborator

@JontySponselee-Naturalis JontySponselee-Naturalis May 4, 2026

Choose a reason for hiding this comment

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

Where does title come from, is it generated by users or external systems? because this can introduce injections vulnerabilities. If it is, then you need to sanitize it

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It is not generated by users, it is either a hardcoded title from me or data that comes from our own API's. Do you recommend sanitizing it then and if so, how?

Copy link
Copy Markdown
Collaborator

@JontySponselee-Naturalis JontySponselee-Naturalis May 6, 2026

Choose a reason for hiding this comment

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

It is fine if it is your input. However, if it comes from your API then it depends where it originally comes from, so in that case I would recommend to santize it.

I have not done this myself, so I can't help much, but I would recommend to use a package since it is security related. But this package seems to be used a lot https://www.npmjs.com/package/dompurify

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Ok, I've updated it with the package you mentioned. I saw it in multiple places as well. So I think it should be good. :)


/**
* Renders a single row of data.
* If the value is missing/null, it returns null to hide the row.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

When is it missing, is this normal behavior, or should it be logged so you can investigate it?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

No, sometimes the data is just not there. Like for instance taxonomic data. It sometimes has a family, a genus etc, but not a species name for instance. Then we just don't show anything. So we do not have to investigate it, and only show the data that is available :)

Comment thread src/components/LabelValuePair/LabelValuePair.tsx Outdated
Comment thread src/components/Hero/Hero.tsx Outdated
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 7, 2026

@MelanieTheLion MelanieTheLion merged commit 3458234 into main May 8, 2026
4 checks passed
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