Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import styled, {css} from 'styled-components';
import clsx from 'clsx';
import {CSSProperties} from 'react';

import {Colors} from './Color';
import styles from './css/ButtonLink.module.css';

type Colors =
| string
Expand All @@ -12,67 +14,46 @@ type Colors =

type Underline = 'never' | 'always' | 'hover';

const fontColor = (color: Colors) => {
if (typeof color === 'string') {
return css`
color: ${color};
`;
}

const {link, hover, active} = color;
return css`
color: ${link};
${hover ? `&:hover { color: ${hover}; }` : null}
${active ? `&:active { color: ${active}; }` : null}
`;
const getColorVars = (color: Colors) => {
const values = typeof color === 'string' ? {link: color, hover: color, active: color} : color;
return {
'--button-link-color': values.link,
'--button-link-hover-color': values.hover,
'--button-link-active-color': values.active,
} as CSSProperties;
};

const textDecoration = (underline: Underline) => {
switch (underline) {
case 'always':
return css`
text-decoration: underline;
`;
case 'hover':
return css`
&:hover:not(:disabled) {
text-decoration: underline;
}
`;
case 'never':
default:
return null;
}
const getTextDecorationClassName = (underline: Underline) => {
return clsx(
underline === 'always' && styles.textDecorationAlways,
underline === 'hover' && styles.textDecorationHover,
);
};

interface Props extends Omit<React.ButtonHTMLAttributes<HTMLButtonElement>, 'color'> {
color?: Colors;
underline?: Underline;
}

export const ButtonLink = styled(({color: _color, underline: _underline, ...rest}: Props) => (
<button {...rest} />
))`
background: transparent;
border: 0;
cursor: pointer;
font-size: inherit;
line-height: 1;
padding: 8px;
margin: -8px;
text-align: left;

&:disabled {
cursor: default;
opacity: 0.7;
}

&:focus:not(:focus-visible) {
outline: none;
}

transition: 30ms color linear;

${({color}) => fontColor(color || Colors.linkDefault())}
${({underline}) => textDecoration(underline || 'hover')}
`;
export const ButtonLink = ({
color,
underline,
style: userStyle,
className: userClassName,
...rest
}: Props) => {
const colors = color ? getColorVars(color) : {};
const textDecoration = underline ? getTextDecorationClassName(underline) : undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

The original implementation defaulted to 'hover' when underline was not provided, but this implementation only applies a class when underline is defined. To maintain the same behavior, consider changing to:

const textDecoration = getTextDecorationClassName(underline || 'hover');

This ensures the hover underline effect remains the default behavior, matching the previous implementation.

Suggested change
const textDecoration = underline ? getTextDecorationClassName(underline) : undefined;
const textDecoration = getTextDecorationClassName(underline || 'hover');

Spotted by Graphite Agent

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

const style = {
...userStyle,
...colors,
};

return (
<button
{...rest}
style={style}
className={clsx(styles.buttonLink, textDecoration, userClassName)}
/>
);
};
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {StoryFn} from '@storybook/nextjs';
import * as React from 'react';

import {ButtonLink} from '../ButtonLink';
import {Colors} from '../Color';
Expand All @@ -25,7 +24,7 @@ ColorMap.args = {
color: {
link: Colors.linkDefault(),
hover: Colors.linkHover(),
active: Colors.linkHover(),
active: Colors.accentGreen(),
},
};

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
.buttonLink {
--button-link-color: var(--color-link-default);
--button-link-hover-color: var(--color-link-default);
--button-link-active-color: var(--color-link-default);
Comment on lines +2 to +4
Copy link
Contributor

Choose a reason for hiding this comment

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

The default values for hover and active colors should be updated to match the original behavior. Currently, all three CSS variables are set to var(--color-link-default), but the hover and active states should use their respective color values from the Colors utility. Specifically, --button-link-hover-color should be set to var(--color-link-hover) to maintain consistency with the previous implementation.

Suggested change
--button-link-color: var(--color-link-default);
--button-link-hover-color: var(--color-link-default);
--button-link-active-color: var(--color-link-default);
--button-link-color: var(--color-link-default);
--button-link-hover-color: var(--color-link-hover);
--button-link-active-color: var(--color-link-active);

Spotted by Graphite Agent

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.


background: transparent;
border: 0;
color: var(--button-link-color);
cursor: pointer;
font-size: inherit;
line-height: 1;
padding: 8px;
margin: -8px;
text-align: left;
transition: 30ms color linear;
}

.buttonLink:disabled {
cursor: default;
opacity: 0.7;
}

.buttonLink:hover {
color: var(--button-link-hover-color);
}

.buttonLink:active {
color: var(--button-link-active-color);
}

.buttonLink:focus:not(:focus-visible) {
outline: none;
}

.textDecorationAlways,
.textDecorationHover:hover:not(:disabled) {
text-decoration: underline;
}