-
Notifications
You must be signed in to change notification settings - Fork 88
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a link to the SpliceAI lookup page on the SpliceAI predictions #3486
Conversation
@@ -69,7 +71,11 @@ const Prediction = ({ field, fieldTitle, value, color, infoValue, infoTitle, war | |||
<div> | |||
{indicator} | |||
{fieldDisplay} | |||
<PredictionValue>{value}</PredictionValue> | |||
{href ? ( | |||
<ButtonLink as="a" href={href} target="_blank" rel="noreferrer"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<ButtonLink as="a"
is just a more complicated way to write <a
. There is no advantage to making this a button as it needs no onclick behavior, so it should just be a link
@@ -13,7 +13,7 @@ import { ButtonLink } from '../../StyledComponents' | |||
const PredictionValue = styled.span` | |||
margin-left: 5px; | |||
font-weight: bolder; | |||
color: black; | |||
color: ${props => props.color}; | |||
text-transform: uppercase; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so this should not be an explicit prop, using an actual link for the value should fix the style issue
<ButtonLink as="a" href={href} target="_blank" rel="noreferrer"> | ||
<PredictionValue>{value}</PredictionValue> | ||
</ButtonLink> | ||
) : <PredictionValue color="black">{value}</PredictionValue>} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is cleaner to wrap the value in the link if needed, instead of rendering PredictionValue
twice:
const valueDisplay = href ? <a href={href} target="_blank" rel="noreferrer">{value}</a> : value
...
<PredictionValue>{valueDisplay}</PredictionValue>
See the connected issue.