Update Link Component #1472
Update Link Component #1472
Conversation
camielvs
force-pushed
the
12-04-update_link_component
branch
from
ecc2b18 to
ebfc92f
Compare
| <TableCell className="text-sm flex items-center gap-2"> | ||
| <StatusIcon status={getRunStatus(statusCounts)} /> | ||
| <Link {...LinkProps}>{name}</Link> | ||
| <Paragraph>{name}</Paragraph> |
There was a problem hiding this comment.
The row already has a handler to navigate to the url, so an embedded link to the same location is unnecessary
| primary: "text-primary hover:underline", | ||
| disabled: "text-muted-foreground cursor-not-allowed pointer-events-none", | ||
| classic: "text-sky-500 hover:text-sky-600 hover:underline", | ||
| block: "text-inherit", |
There was a problem hiding this comment.
for when the link wraps around a block of content such as a table row or an icon
| > | ||
| {children} | ||
| {external && <ExternalLink className={cn("h-4", iconClassName)} />} | ||
| {download && <DownloadIcon className={cn("h-4", iconClassName)} />} |
| <footer | ||
| className="footer w-full px-4 text-center bg-gray-50" | ||
| style={{ height: `${BOTTOM_FOOTER_HEIGHT}px` }} | ||
| > |
There was a problem hiding this comment.
I know you didn't change it but can we make the footer component? I noticed, for some reason, these changes made the text on the footer sit a little high and this fixed it.
<footer
className={`footer w-full px-4 text-center items-center bg-grey-50 h-[${BOTTOM_FOOTER_HEIGHT}px]`}
>There was a problem hiding this comment.
Are you sure? The text looks fine to me. Do you have a screenshot?
From my understanding of Tailwind embedding a variable into the tailwind string doesn't work.
I am going to merge and if this truly is an issue we can fix separately.
There was a problem hiding this comment.
There was a problem hiding this comment.
its small but this is what I was seeing
There was a problem hiding this comment.
I think this is inadvertently fixed by #1484
camielvs
force-pushed
the
12-04-update_link_component
branch
from
ebfc92f to
ed53591
Compare
Description
Update our core
Linkcomponent for improved UI. Migrate various places around the app onto the Link component.Related Issue and Pull requests
Type of Change
Checklist
Screenshots (if applicable)
Test Instructions
All links should work as expected!
Additional Comments
Given I passed through a variety of components I made an effort NOT to start refactoring everything I saw onto UI primitives. I will deal with that later. This PR is solely focussed on
<a />and<Link />