Skip to content

Commit

Permalink
OPHJOD-281: Remove classnames and simplify dropdown menu
Browse files Browse the repository at this point in the history
  • Loading branch information
sauanto committed Mar 26, 2024
1 parent c1b884d commit d296a84
Show file tree
Hide file tree
Showing 8 changed files with 111 additions and 106 deletions.
17 changes: 5 additions & 12 deletions lib/components/Button/Button.stories.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { Meta, StoryObj } from '@storybook/react';
import { fn } from '@storybook/test';

import { Button } from './Button';

Expand All @@ -16,38 +17,30 @@ export const Base: Story = {
args: {
label: 'Base Button',
variant: 'base',
onClick: () => {
// Do nothing
},
onClick: fn(),
},
};

export const Primary: Story = {
args: {
label: 'Primary Button',
variant: 'primary',
onClick: () => {
// Do nothing
},
onClick: fn(),
},
};

export const Outlined: Story = {
args: {
label: 'Outlined Button',
variant: 'outlined',
onClick: () => {
// Do nothing
},
onClick: fn(),
},
};

export const Text: Story = {
args: {
label: 'Text Button',
variant: 'text',
onClick: () => {
// Do nothing
},
onClick: fn(),
},
};
33 changes: 14 additions & 19 deletions lib/components/Button/Button.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
import classNames from 'classnames';
/**
* base = default button to be used
* primary = should be only appear once in a page
* text = for less-pronounced actions
*/
export type ButtonVariant = 'base' | 'primary' | 'outlined' | 'text';
export type ButtonVariant =
| 'base' // default button to be used
| 'primary' // should be only appear once in a page
| 'outlined'
| 'text'; // for less-pronounced actions

export interface ButtonProps {
/** Text shown on the button */
Expand All @@ -18,19 +16,16 @@ export interface ButtonProps {
}

export const Button = ({ label, onClick, variant = 'base', disabled = false }: ButtonProps) => {
const className = `m-2 px-4 py-2 font-bold
${variant === 'base' ? 'bg-jod-base text-jod-white' : ''}
${variant === 'primary' ? 'bg-jod-primary text-jod-white' : ''}
${variant === 'outlined' ? 'border border-jod-black text-jod-black' : ''}
${variant === 'text' ? 'text-jod-black' : ''}
${disabled ? 'cursor-not-allowed opacity-50' : ''}`
.replace(/\s+/g, ' ')
.trim();
return (
<button
disabled={disabled}
type="button"
onClick={onClick}
className={classNames('jod-button', 'm-2 px-4 py-2 font-bold', {
'bg-jod-base text-jod-white': variant === 'base',
'bg-jod-primary text-jod-white': variant === 'primary',
'border border-jod-black text-jod-black': variant === 'outlined',
'text-jod-black': variant === 'text',
'cursor-not-allowed opacity-50': disabled,
})}
>
<button disabled={disabled} type="button" onClick={onClick} className={className}>
{label}
</button>
);
Expand Down
8 changes: 4 additions & 4 deletions lib/components/Button/__snapshots__/Button.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

exports[`Snapshot testing > Base button 1`] = `
<button
class="jod-button m-2 px-4 py-2 font-bold bg-jod-base text-jod-white"
class="m-2 px-4 py-2 font-bold bg-jod-base text-jod-white"
type="button"
>
Base button
Expand All @@ -11,7 +11,7 @@ exports[`Snapshot testing > Base button 1`] = `

exports[`Snapshot testing > Outlined button 1`] = `
<button
class="jod-button m-2 px-4 py-2 font-bold border border-jod-black text-jod-black"
class="m-2 px-4 py-2 font-bold border border-jod-black text-jod-black"
type="button"
>
Outlined button
Expand All @@ -20,7 +20,7 @@ exports[`Snapshot testing > Outlined button 1`] = `

exports[`Snapshot testing > Primary button 1`] = `
<button
class="jod-button m-2 px-4 py-2 font-bold bg-jod-primary text-jod-white"
class="m-2 px-4 py-2 font-bold bg-jod-primary text-jod-white"
type="button"
>
Primary button
Expand All @@ -29,7 +29,7 @@ exports[`Snapshot testing > Primary button 1`] = `

exports[`Snapshot testing > Text button 1`] = `
<button
class="jod-button m-2 px-4 py-2 font-bold text-jod-black"
class="m-2 px-4 py-2 font-bold text-jod-black"
type="button"
>
Text button
Expand Down
8 changes: 4 additions & 4 deletions lib/components/DropdownMenu/DropdownMenu.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,14 @@ describe('defaultValue', () => {
});

it('should select the wanted one', () => {
render(<DropdownMenu label="" options={options} defaultValue="sv" />);
render(<DropdownMenu label="" options={options} defaultValue={options[1]} />);
// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
expect((screen.getByRole('option', { name: 'Svenska' }) as HTMLOptionElement).selected).toBe(true);
//
});

it('should still allow user to another', async () => {
render(<DropdownMenu label="" options={options} defaultValue="fi" />);
render(<DropdownMenu label="" options={options} defaultValue={options[0]} />);
const user = userEvent.setup();
await user.selectOptions(screen.getByRole('combobox'), 'English');

Expand Down Expand Up @@ -91,8 +91,8 @@ describe('label', () => {
});

describe('hideLabel', () => {
it('should add correct CSS class to label', () => {
it('should add aria-label when label is hidden', () => {
render(<DropdownMenu label="Non-visible label" options={[]} hideLabel={true} />);
expect(screen.getByText('Non-visible label').classList).toContain('sr-only');
expect(screen.getByRole('combobox', { name: 'Non-visible label' })).not.toBeNull();
});
});
116 changes: 74 additions & 42 deletions lib/components/DropdownMenu/DropdownMenu.tsx
Original file line number Diff line number Diff line change
@@ -1,72 +1,104 @@
import { useId } from 'react';
import classNames from 'classnames';

export interface DropdownMenuOptionsData<T extends string = string> {
value: T;
export interface DropdownMenuSelectProps<T extends DropdownMenuOptionsData> {
/** Unique id for the component */
id: string;
/** Component is disabled for user interaction */
disabled?: boolean;
/** Aria label for screenreaders */
ariaLabel?: string;
/** Hide label. Still available for screenreaders */
hideLabel?: boolean;
/** Options for component */
options: T[];
/** Default value to be selected initially */
defaultValue?: T;
/** Controlled mode */
selected?: T;
/** Callback on selection change */
onChange?: (value: T) => void;
}

const DropdownMenuSelect = <T extends DropdownMenuOptionsData = DropdownMenuOptionsData>({
id,
disabled,
ariaLabel,
onChange,
defaultValue,
options,
}: DropdownMenuSelectProps<T>) => (
<select
id={id}
disabled={disabled}
aria-label={ariaLabel}
className="min-w-[120px] justify-self-end rounded-lg border border-jod-dark bg-jod-white p-2 hover:bg-purple-100 focus:outline-none focus:ring focus:ring-purple-500 disabled:border-gray-500 disabled:bg-gray-200 disabled:text-gray-500 disabled:hover:bg-gray-200"
onChange={(event: React.ChangeEvent<HTMLSelectElement>) => {
if (onChange) {
onChange(options.find((option) => option.value === event.target.value)!);
}
}}
defaultValue={defaultValue?.value}
>
{options.map((option, index) => (
<option key={index} value={option.value}>
{option.label}
</option>
))}
</select>
);

export interface DropdownMenuOptionsData {
value: string;
label: string;
}

interface DropdownMenuProps<T extends DropdownMenuOptionsData, U extends string = string> {
interface DropdownMenuProps<T extends DropdownMenuOptionsData> {
/** Label for the component */
label: string;
/** Hide label. Still available for screenreaders */
hideLabel?: boolean;
/** Options for component */
options: T[];
/** Default value to be selected initially */
defaultValue?: U;
defaultValue?: T;
/** Controlled mode */
selected?: U;
selected?: T;
/** Callback on selection change */
onChange?: (value: U) => void;
onChange?: (value: T) => void;
/** Component is disabled for user interaction */
disabled?: boolean;
}

export const DropdownMenu = <
U extends string = string,
T extends DropdownMenuOptionsData<string> = DropdownMenuOptionsData<string>,
>({
export const DropdownMenu = <T extends DropdownMenuOptionsData = DropdownMenuOptionsData>({
label,
hideLabel = false,
options,
defaultValue,
onChange: propOnChange,
onChange,
disabled = false,
}: DropdownMenuProps<T, U>) => {
const labelId = useId();
return (
<div className="flex flex-row">
<label
htmlFor={labelId}
className={classNames('mr-2 self-center font-bold text-jod-black', {
'sr-only': hideLabel,
})}
>
}: DropdownMenuProps<T>) => {
const selectId = useId();
return hideLabel ? (
<DropdownMenuSelect
id={selectId}
disabled={disabled}
ariaLabel={label}
onChange={onChange}
defaultValue={defaultValue}
options={options}
/>
) : (
<div className="flex items-center gap-2">
<label htmlFor={selectId} className="font-bold text-jod-black">
{label}
</label>
<select
<DropdownMenuSelect
id={selectId}
disabled={disabled}
id={labelId}
className={classNames(
'min-w-[120px] justify-self-end rounded-lg border border-jod-dark bg-jod-white p-2 hover:bg-purple-100 focus:outline-none focus:ring focus:ring-purple-500',
{
'disabled:border-gray-500 disabled:bg-gray-200 disabled:text-gray-500 disabled:hover:bg-gray-200': disabled,
},
)}
onChange={(event: React.ChangeEvent<HTMLSelectElement>) => {
if (propOnChange) {
propOnChange(event.target.value as U);
}
}}
onChange={onChange}
defaultValue={defaultValue}
>
{options.map((option) => (
<option key={option.value} value={option.value}>
{option.label}
</option>
))}
</select>
options={options}
/>
</div>
);
};
Original file line number Diff line number Diff line change
@@ -1,26 +1,19 @@
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html

exports[`Snapshot testing > should not show label 1`] = `
<div
class="flex flex-row"
>
<label
class="mr-2 self-center font-bold text-jod-black sr-only"
for=":r1:"
/>
<select
class="min-w-[120px] justify-self-end rounded-lg border border-jod-dark bg-jod-white p-2 hover:bg-purple-100 focus:outline-none focus:ring focus:ring-purple-500"
id=":r1:"
/>
</div>
<select
aria-label=""
class="min-w-[120px] justify-self-end rounded-lg border border-jod-dark bg-jod-white p-2 hover:bg-purple-100 focus:outline-none focus:ring focus:ring-purple-500 disabled:border-gray-500 disabled:bg-gray-200 disabled:text-gray-500 disabled:hover:bg-gray-200"
id=":r1:"
/>
`;

exports[`Snapshot testing > should render as disabled 1`] = `
<div
class="flex flex-row"
class="flex items-center gap-2"
>
<label
class="mr-2 self-center font-bold text-jod-black"
class="font-bold text-jod-black"
for=":r2:"
/>
<select
Expand All @@ -33,14 +26,14 @@ exports[`Snapshot testing > should render as disabled 1`] = `

exports[`Snapshot testing > should render with defaults 1`] = `
<div
class="flex flex-row"
class="flex items-center gap-2"
>
<label
class="mr-2 self-center font-bold text-jod-black"
class="font-bold text-jod-black"
for=":r0:"
/>
<select
class="min-w-[120px] justify-self-end rounded-lg border border-jod-dark bg-jod-white p-2 hover:bg-purple-100 focus:outline-none focus:ring focus:ring-purple-500"
class="min-w-[120px] justify-self-end rounded-lg border border-jod-dark bg-jod-white p-2 hover:bg-purple-100 focus:outline-none focus:ring focus:ring-purple-500 disabled:border-gray-500 disabled:bg-gray-200 disabled:text-gray-500 disabled:hover:bg-gray-200"
id=":r0:"
/>
</div>
Expand Down
7 changes: 0 additions & 7 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
"@vitejs/plugin-react-swc": "^3.5.0",
"@vitest/coverage-v8": "^1.2.1",
"autoprefixer": "^10.4.18",
"classnames": "^2.5.1",
"eslint": "^8.56.0",
"eslint-config-prettier": "^9.1.0",
"eslint-plugin-jsx-a11y": "^6.8.0",
Expand Down

0 comments on commit d296a84

Please sign in to comment.