Skip to content
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

feat(radio)!: remove top level subcomponents #1690

Merged
merged 2 commits into from
Jul 17, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
85 changes: 85 additions & 0 deletions src/components/Radio/Radio.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,88 @@
display: flex;
gap: var(--eds-size-1);
}

/**
* Wraps the visually hidden radio input element and the visible sibling svg element.
*/
.input__wrapper {
position: relative;
/* Centers the radio icon in the wrapper. */
display: inline-flex;
align-items: center;
/* Aligns the radio with the first line of the label. */
align-self: flex-start;
}
/**
* The visually hidden input element for the radio. The visual radio is provided by an svg element.
*/
.radio__input {
/* Removes default margins placed by browser for radioes. */
margin: 0;
/* Remove the radio from the page flow, positioning it on top of the SVG. */
position: absolute;
left: 0.25rem;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this should be --eds-size-half i think but I'm guessing this was just brought over from the other file

/* Set same dimensions as the RadioSvg element. */
height: var(--eds-size-2);
width: var(--eds-size-2);
/**
* Hide the input element visually.
* This ensures the radio is still "physically" present so that all users,
* especially on touch screen readers, still interact with the real radio element
* where it would naturally be present.
*/
opacity: 0;
}

/**
* The visible radio svg icon element.
*/
.radio__icon {
color: var(--eds-theme-color-radio-brand-background);
/* Creates space for the border so that there's no jitter when the focus border is visible. */
border: var(--eds-border-width-sm) solid transparent;
}
.radio__input:focus-visible + .radio__icon {
border: var(--eds-border-width-sm) solid var(--eds-theme-color-focus-ring);
border-radius: var(--eds-border-radius-full);
}

@supports not selector(:focus-visible) {
.radio__input:focus + .radio__icon {
border: var(--eds-border-width-sm) solid var(--eds-theme-color-focus-ring);
border-radius: var(--eds-border-radius-full);
}
}

/**
* The disabled variant of the visible radio svg icon.
*/
.radio__icon--disabled {
color: var(--eds-theme-color-icon-disabled);
}

/**
* The disabled status of the visually hidden input element.
*/
.radio__input:disabled {
/* Needed since the input element overlays the visible svg icon for user input and cursor. */
cursor: not-allowed;
}

/**
* Text that labels a radio input.
*/
.radio__label {
position: relative;
}

/**
* Radio label size variants.
* Used for centering the label with the radio input button.
*/
.radio__label--md {
top: 0.125rem;
}
.radio__label--lg {
top: 0.0625rem;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a fan of this, but again this was just brought over :)

Copy link
Contributor

Choose a reason for hiding this comment

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

0.125rem corresponds to --eds-size-quarter which does exist, but --eds-size-eighth or what have you does not...and probably shouldn't. the sizes tokens are kinda weird. I wonder if there are other cases of such a small relative spacing that would indicate we should add this token?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image
There are a some instances where 0.0625rem is used; still tho eighth is kind of small?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll convert the instances of 0.25rem and 0.125rem to the size tokens and create a ticket to think about using eds-size-eighth if that's alright with you @booc0mtaco

Copy link
Contributor

Choose a reason for hiding this comment

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

very small... one pixel in fact :/

This seems to exist to vertically center the text with the radio button. there may be a way to do the same via flex and avoid this kinda thing altogether. So as not to over-complicate this PR, we can make a comment on why such a precise value is being used, then leave it be for now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense to me, will do

90 changes: 84 additions & 6 deletions src/components/Radio/Radio.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,11 @@ import clsx from 'clsx';
import React from 'react';
import { useId } from '../../util/useId';
import type { EitherInclusive } from '../../util/utility-types';
import type { RadioInputProps } from '../RadioInput';
import RadioInput from '../RadioInput';
import type { RadioLabelProps } from '../RadioLabel';
import RadioLabel from '../RadioLabel';
import Icon from '../Icon';
import { InputLabel, type InputLabelProps } from '../InputLabel/InputLabel';
import styles from './Radio.module.css';

export type RadioProps = RadioInputProps & {
type RadioProps = RadioInputProps & {
/**
* HTML id attribute. If not passed, this component
* will generate an id to use for accessibility.
Expand All @@ -17,7 +15,7 @@ export type RadioProps = RadioInputProps & {
/**
* Size of the radio label.
*/
size?: RadioLabelProps['size'];
size?: InputLabelProps['size'];
} & EitherInclusive<
{
/**
Expand All @@ -32,6 +30,86 @@ export type RadioProps = RadioInputProps & {
'aria-label': string;
}
>;
type RadioInputProps = Omit<
React.InputHTMLAttributes<HTMLInputElement>,
'id' | 'size'
> & {
/**
* Additional classnames passed in for styling.
*/
className?: string;
/**
* Radio ID. Used to connect the input with a label for accessibility purposes.
*/
id?: string;
};

const RadioSvg = ({
checked,
disabled,
}: {
checked?: boolean;
disabled?: boolean;
}) => {
const iconClassName = clsx(
styles['radio__icon'],
disabled && styles['radio__icon--disabled'],
);
return (
<Icon
className={iconClassName}
name={checked ? 'radio-selected' : 'radio-unselected'}
purpose="decorative"
size="1.625rem"
/>
);
};

/**
* Radio input element, exported for greater flexibility.
* You must provide an `id` prop and connect it to a visible label.
*/
export const RadioInput = ({
checked = false,
className,
disabled,
...other
}: RadioInputProps) => {
return (
<span
className={clsx(
styles['input__wrapper'],
disabled && styles['input__wrapper--disabled'],
)}
>
<input
checked={checked}
className={clsx(className, styles['radio__input'])}
disabled={disabled}
type="radio"
{...other}
/>
<RadioSvg checked={checked} disabled={disabled} />
</span>
);
};

/**
* Radio label element, styles and relies on the InputLabel component.
*/
export const RadioLabel = ({
className,
size = 'lg',
...other
}: InputLabelProps) => {
const componentClassName = clsx(
styles['radio__label'],
styles[`radio__label--${size}`],
className,
);

return <InputLabel className={componentClassName} size={size} {...other} />;
};

/**
* `import {Radio} from "@chanzuckerberg/eds";`
Expand Down
70 changes: 0 additions & 70 deletions src/components/RadioInput/RadioInput.module.css

This file was deleted.

68 changes: 0 additions & 68 deletions src/components/RadioInput/RadioInput.tsx

This file was deleted.

2 changes: 0 additions & 2 deletions src/components/RadioInput/index.ts

This file was deleted.

21 changes: 0 additions & 21 deletions src/components/RadioLabel/RadioLabel.module.css

This file was deleted.

24 changes: 0 additions & 24 deletions src/components/RadioLabel/RadioLabel.tsx

This file was deleted.

2 changes: 0 additions & 2 deletions src/components/RadioLabel/index.ts

This file was deleted.

Loading