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

fix(progressbar): fix unclear label usage (#468) #547

Merged
merged 11 commits into from
Mar 5, 2023
18 changes: 14 additions & 4 deletions src/docs/pages/ProgressPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,22 @@ const ProgressPage: FC = () => {
),
},
{
title: 'With label inside',
code: <Progress progress={50} label="Flowbite" labelPosition="inside" size="lg" />,
title: 'With labels',
code: <Progress progress={50} labelProgress={true} textLabel="Flowbite" labelText={true} size="lg" />,
},
{
title: 'With label outside',
code: <Progress progress={45} label="Flowbite" labelPosition="outside" labelProgress={true} />,
title: 'Label positions',
code: (
<Progress
progress={45}
labelProgress={true}
progressLabelPosition="inside"
textLabel="Flowbite"
labelText={true}
textLabelPosition="outside"
size="lg"
/>
),
},
];

Expand Down
50 changes: 47 additions & 3 deletions src/lib/components/Progress/Progress.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ import { Progress } from './Progress';
describe.concurrent('Components / Progress', () => {
describe.concurrent('A11y', () => {
it('should have `role="progressbar"`', () => {
render(<Progress label="Accessible name" progress={45} />);
render(<Progress textLabel="Accessible name" progress={45} />);

expect(progressBar()).toBeInTheDocument();
});

it('should use `label` as accessible name', () => {
render(<Progress label="Accessible name" labelPosition="outside" labelProgress progress={45} />);
it('should use `textLabel` as accessible name', () => {
render(<Progress textLabel="Accessible name" textLabelPosition="outside" labelProgress progress={45} />);

expect(progressBar()).toHaveAccessibleName('Accessible name');
});
Expand All @@ -21,7 +21,51 @@ describe.concurrent('Components / Progress', () => {

expect(progressBar()).toHaveAttribute('aria-valuenow', '45');
});

it('should only display labels if specified', () => {
render(<Progress progress={45} labelProgress={false} textLabel="Flowbite" labelText={true} />);

expect(progressBar()).not.toHaveTextContent('45');
expect(progressBar()).toHaveTextContent('Flowbite');
});

it('should display test label inside, progress label outside', () => {
render(
<Progress
progress={45}
labelProgress={true}
progressLabelPosition="outside"
textLabel="Flowbite"
labelText={true}
/>,
);

expect(outerLabelContainer()).toBeInTheDocument();
expect(outerProgressLabel()).toHaveTextContent('45%');
expect(innerTextLabel()).toHaveTextContent('Flowbite');
});

it('should display text label outside, progress label inside', () => {
render(
<Progress
progress={45}
labelProgress={true}
textLabel="Flowbite"
labelText={true}
textLabelPosition="outside"
/>,
);

expect(outerLabelContainer()).toBeInTheDocument();
expect(outerTextLabel()).toHaveTextContent('Flowbite');
expect(innerProgressLabel()).toHaveTextContent('45%');
});
});
});

const progressBar = () => screen.getByRole('progressbar');
const outerLabelContainer = () => screen.getByTestId('flowbite-progress-outer-label-container');
const outerProgressLabel = () => screen.getByTestId('flowbite-progress-outer-progress-label');
const outerTextLabel = () => screen.getByTestId('flowbite-progress-outer-text-label');
const innerTextLabel = () => screen.getByTestId('flowbite-progress-inner-text-label');
const innerProgressLabel = () => screen.getByTestId('flowbite-progress-inner-progress-label');
17 changes: 11 additions & 6 deletions src/lib/components/Progress/Progress.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,18 +54,23 @@ export const Colors = (): JSX.Element => (
);

export const WithLabelInside = Template.bind({});
WithLabelInside.storyName = 'With label inside';
WithLabelInside.storyName = 'With labels';
WithLabelInside.args = {
label: 'Flowbite',
textLabel: 'Flowbite',
labelText: true,
progress: 45,
labelProgress: true,
size: 'lg',
};

export const WithLabelOutside = Template.bind({});
WithLabelOutside.storyName = 'With label outside';
WithLabelOutside.storyName = 'Label positions';
WithLabelOutside.args = {
label: 'Flowbite',
labelPosition: 'outside',
labelProgress: true,
textLabel: 'Flowbite',
labelText: true,
textLabelPosition: 'outside',
progress: 45,
labelProgress: true,
progressLabelPosition: 'inside',
size: 'lg',
};
36 changes: 25 additions & 11 deletions src/lib/components/Progress/Progress.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,22 +24,26 @@ export interface ProgressSizes extends Pick<FlowbiteSizes, 'sm' | 'md' | 'lg' |
}

export interface ProgressProps extends PropsWithChildren, ComponentProps<'div'> {
size?: keyof ProgressSizes;
label?: string;
labelPosition?: 'inside' | 'outside' | 'none';
labelProgress?: boolean;
labelText?: boolean;
progress: number;
progressLabelPosition?: 'inside' | 'outside';
size?: keyof ProgressSizes;
textLabel?: string;
textLabelPosition?: 'inside' | 'outside';
theme?: DeepPartial<FlowbiteProgressTheme>;
}

export const Progress: FC<ProgressProps> = ({
className,
color = 'blue',
label = 'progressbar',
labelPosition = 'none',
labelProgress = false,
labelText = false,
progress,
progressLabelPosition = 'inside',
size = 'md',
textLabel = 'progressbar',
textLabelPosition = 'inside',
theme: customTheme = {},
...props
}) => {
Expand All @@ -48,19 +52,29 @@ export const Progress: FC<ProgressProps> = ({

return (
<>
<div aria-label={label} aria-valuenow={progress} id={id} role="progressbar" {...props}>
{label && labelPosition === 'outside' && (
<div className={theme.label}>
<span>{label}</span>
{labelProgress && <span>{progress}%</span>}
<div id={id} aria-label={textLabel} aria-valuenow={progress} role="progressbar" {...props}>
{((textLabel && labelText && textLabelPosition === 'outside') ||
(progress && labelProgress && progressLabelPosition === 'outside')) && (
<div className={theme.label} data-testid="flowbite-progress-outer-label-container">
{textLabel && labelText && textLabelPosition === 'outside' && (
<span data-testid="flowbite-progress-outer-text-label">{textLabel}</span>
)}
{progress && labelProgress && progressLabelPosition === 'outside' && (
<span data-testid="flowbite-progress-outer-progress-label">{progress}%</span>
)}
</div>
)}
<div className={classNames(theme.base, theme.size[size], className)}>
<div
style={{ width: `${progress}%` }}
className={classNames(theme.bar, theme.color[color], theme.size[size])}
>
{label && labelPosition === 'inside' && label}
{textLabel && labelText && textLabelPosition === 'inside' && (
<span data-testid="flowbite-progress-inner-text-label">{textLabel}</span>
)}
{progress && labelProgress && progressLabelPosition === 'inside' && (
<span data-testid="flowbite-progress-inner-progress-label">{progress}%</span>
)}
</div>
</div>
</div>
Expand Down
2 changes: 1 addition & 1 deletion src/lib/theme/default.ts
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,7 @@ const theme: FlowbiteTheme = {
progress: {
base: 'w-full overflow-hidden rounded-full bg-gray-200 dark:bg-gray-700',
label: 'mb-1 flex justify-between font-medium dark:text-white',
bar: 'flex items-center justify-center rounded-full text-center font-medium leading-none text-blue-100',
bar: 'flex items-center justify-center rounded-full text-center font-medium leading-none text-blue-100 space-x-2',
color: {
dark: 'bg-gray-600 dark:bg-gray-300',
blue: 'bg-blue-600',
Expand Down