From bcbf30929f9ecb82804528be8bd160530084f2b8 Mon Sep 17 00:00:00 2001 From: revnandi Date: Wed, 18 Jan 2023 17:00:28 +0100 Subject: [PATCH 1/6] fix(progressbar): fix unclear label usage (#468) --- src/docs/pages/ProgressPage.tsx | 18 +++++++++--- src/lib/components/Progress/Progress.spec.tsx | 13 +++++++-- .../components/Progress/Progress.stories.tsx | 17 +++++++---- src/lib/components/Progress/Progress.tsx | 28 +++++++++++-------- 4 files changed, 52 insertions(+), 24 deletions(-) diff --git a/src/docs/pages/ProgressPage.tsx b/src/docs/pages/ProgressPage.tsx index 7c9a53d85..fe6fbf337 100644 --- a/src/docs/pages/ProgressPage.tsx +++ b/src/docs/pages/ProgressPage.tsx @@ -46,12 +46,22 @@ const ProgressPage: FC = () => { ), }, { - title: 'With label inside', - code: , + title: 'With labels', + code: , }, { - title: 'With label outside', - code: , + title: 'Label positions', + code: ( + + ), }, ]; diff --git a/src/lib/components/Progress/Progress.spec.tsx b/src/lib/components/Progress/Progress.spec.tsx index 549deb8af..772f91142 100644 --- a/src/lib/components/Progress/Progress.spec.tsx +++ b/src/lib/components/Progress/Progress.spec.tsx @@ -5,13 +5,13 @@ import { Progress } from './Progress'; describe.concurrent('Components / Progress', () => { describe.concurrent('A11y', () => { it('should have `role="progressbar"`', () => { - render(); + render(); expect(progressBar()).toBeInTheDocument(); }); - it('should use `label` as accessible name', () => { - render(); + it('should use `textLabel` as accessible name', () => { + render(); expect(progressBar()).toHaveAccessibleName('Accessible name'); }); @@ -21,6 +21,13 @@ describe.concurrent('Components / Progress', () => { expect(progressBar()).toHaveAttribute('aria-valuenow', '45'); }); + + it('should only display labels if specified', () => { + render(); + + expect(progressBar()).not.toHaveTextContent('45'); + expect(progressBar()).toHaveTextContent('Flowbite'); + }); }); }); diff --git a/src/lib/components/Progress/Progress.stories.tsx b/src/lib/components/Progress/Progress.stories.tsx index a88ecce9a..e6a986a16 100644 --- a/src/lib/components/Progress/Progress.stories.tsx +++ b/src/lib/components/Progress/Progress.stories.tsx @@ -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', }; diff --git a/src/lib/components/Progress/Progress.tsx b/src/lib/components/Progress/Progress.tsx index 3c93fb366..726f77af4 100644 --- a/src/lib/components/Progress/Progress.tsx +++ b/src/lib/components/Progress/Progress.tsx @@ -23,18 +23,22 @@ export interface ProgressSizes extends Pick> { size?: keyof ProgressSizes; - label?: string; - labelPosition?: 'inside' | 'outside' | 'none'; - labelProgress?: boolean; + textLabel?: string; + labelText?: boolean; + textLabelPosition?: 'inside' | 'outside'; progress: number; + labelProgress?: boolean; + progressLabelPosition?: 'inside' | 'outside'; } export const Progress: FC = ({ color = 'blue', - label = 'progressbar', - labelPosition = 'none', - labelProgress = false, + textLabel = 'progressbar', + labelText = false, + textLabelPosition = 'inside', progress, + labelProgress = false, + progressLabelPosition = 'inside', size = 'md', className, ...props @@ -44,11 +48,12 @@ export const Progress: FC = ({ return ( <> -
- {label && labelPosition === 'outside' && ( +
+ {((textLabel && labelText && textLabelPosition === 'outside') || + (progress && labelProgress && progressLabelPosition === 'outside')) && (
- {label} - {labelProgress && {progress}%} + {textLabel && labelText && textLabelPosition === 'outside' && {textLabel}} + {progress && labelProgress && progressLabelPosition === 'outside' && {progress}%}
)}
@@ -56,7 +61,8 @@ export const Progress: FC = ({ className={classNames(theme.bar, theme.color[color], theme.size[size])} style={{ width: `${progress}%` }} > - {label && labelPosition === 'inside' && label} + {textLabel && labelText && textLabelPosition === 'inside' && {textLabel}} + {progress && labelProgress && progressLabelPosition === 'inside' && {progress}}
From b84dc1df3d637532884322c5706fe05449ddf0e9 Mon Sep 17 00:00:00 2001 From: revnandi Date: Wed, 18 Jan 2023 17:18:51 +0100 Subject: [PATCH 2/6] fix: add spacing to labels --- src/lib/theme/default.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/theme/default.ts b/src/lib/theme/default.ts index 75c9c7159..02e2389a0 100644 --- a/src/lib/theme/default.ts +++ b/src/lib/theme/default.ts @@ -803,7 +803,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', From bba6d2cf914ab4068b8d4dfa53bdcc919b7023d3 Mon Sep 17 00:00:00 2001 From: revnandi Date: Thu, 19 Jan 2023 00:46:03 +0100 Subject: [PATCH 3/6] add percentage symbol to inside progress label --- src/lib/components/Progress/Progress.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/components/Progress/Progress.tsx b/src/lib/components/Progress/Progress.tsx index 726f77af4..67e501143 100644 --- a/src/lib/components/Progress/Progress.tsx +++ b/src/lib/components/Progress/Progress.tsx @@ -62,7 +62,7 @@ export const Progress: FC = ({ style={{ width: `${progress}%` }} > {textLabel && labelText && textLabelPosition === 'inside' && {textLabel}} - {progress && labelProgress && progressLabelPosition === 'inside' && {progress}} + {progress && labelProgress && progressLabelPosition === 'inside' && {progress}%} From 38ea41e28713693b6fa12ce5530ac383f1964e13 Mon Sep 17 00:00:00 2001 From: revnandi Date: Thu, 19 Jan 2023 16:43:18 +0100 Subject: [PATCH 4/6] add unit tests --- src/lib/components/Progress/Progress.spec.tsx | 11 +++++++++++ src/lib/components/Progress/Progress.tsx | 13 ++++++++----- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/lib/components/Progress/Progress.spec.tsx b/src/lib/components/Progress/Progress.spec.tsx index 772f91142..84c520193 100644 --- a/src/lib/components/Progress/Progress.spec.tsx +++ b/src/lib/components/Progress/Progress.spec.tsx @@ -28,7 +28,18 @@ describe.concurrent('Components / Progress', () => { expect(progressBar()).not.toHaveTextContent('45'); expect(progressBar()).toHaveTextContent('Flowbite'); }); + + it('should display labels in specified positions', () => { + render(); + + expect(outerLabelContainer()).toBeInTheDocument(); + expect(outerProgressLabel()).toHaveTextContent('45'); + expect(innerTextLabel()).toHaveTextContent('Flowbite'); + }); }); }); const progressBar = () => screen.getByRole('progressbar'); +const outerLabelContainer = () => screen.getByTestId('flowbite-progress-outer-label-container'); +const outerProgressLabel = () => screen.getByTestId('flowbite-progress-outer-progress-label'); +const innerTextLabel = () => screen.getByTestId('flowbite-progress-inner-text-label'); diff --git a/src/lib/components/Progress/Progress.tsx b/src/lib/components/Progress/Progress.tsx index 67e501143..8042d744b 100644 --- a/src/lib/components/Progress/Progress.tsx +++ b/src/lib/components/Progress/Progress.tsx @@ -51,9 +51,12 @@ export const Progress: FC = ({
{((textLabel && labelText && textLabelPosition === 'outside') || (progress && labelProgress && progressLabelPosition === 'outside')) && ( -
- {textLabel && labelText && textLabelPosition === 'outside' && {textLabel}} - {progress && labelProgress && progressLabelPosition === 'outside' && {progress}%} +
+ {textLabel && labelText && textLabelPosition === 'outside' && {textLabel}} + {progress && labelProgress && progressLabelPosition === 'outside' && {progress}%}
)}
@@ -61,8 +64,8 @@ export const Progress: FC = ({ className={classNames(theme.bar, theme.color[color], theme.size[size])} style={{ width: `${progress}%` }} > - {textLabel && labelText && textLabelPosition === 'inside' && {textLabel}} - {progress && labelProgress && progressLabelPosition === 'inside' && {progress}%} + {textLabel && labelText && textLabelPosition === 'inside' && {textLabel}} + {progress && labelProgress && progressLabelPosition === 'inside' && {progress}%}
From 8660f1e674688a06b6efab4191b7424977ccc66a Mon Sep 17 00:00:00 2001 From: revnandi Date: Thu, 19 Jan 2023 21:39:42 +0100 Subject: [PATCH 5/6] fix code formatting --- src/lib/components/Progress/Progress.spec.tsx | 10 ++++++++- src/lib/components/Progress/Progress.tsx | 21 ++++++++++++------- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/lib/components/Progress/Progress.spec.tsx b/src/lib/components/Progress/Progress.spec.tsx index 84c520193..7a6d0e4d9 100644 --- a/src/lib/components/Progress/Progress.spec.tsx +++ b/src/lib/components/Progress/Progress.spec.tsx @@ -30,7 +30,15 @@ describe.concurrent('Components / Progress', () => { }); it('should display labels in specified positions', () => { - render(); + render( + , + ); expect(outerLabelContainer()).toBeInTheDocument(); expect(outerProgressLabel()).toHaveTextContent('45'); diff --git a/src/lib/components/Progress/Progress.tsx b/src/lib/components/Progress/Progress.tsx index 8042d744b..82d9b8d01 100644 --- a/src/lib/components/Progress/Progress.tsx +++ b/src/lib/components/Progress/Progress.tsx @@ -51,12 +51,13 @@ export const Progress: FC = ({
{((textLabel && labelText && textLabelPosition === 'outside') || (progress && labelProgress && progressLabelPosition === 'outside')) && ( -
- {textLabel && labelText && textLabelPosition === 'outside' && {textLabel}} - {progress && labelProgress && progressLabelPosition === 'outside' && {progress}%} +
+ {textLabel && labelText && textLabelPosition === 'outside' && ( + {textLabel} + )} + {progress && labelProgress && progressLabelPosition === 'outside' && ( + {progress}% + )}
)}
@@ -64,8 +65,12 @@ export const Progress: FC = ({ className={classNames(theme.bar, theme.color[color], theme.size[size])} style={{ width: `${progress}%` }} > - {textLabel && labelText && textLabelPosition === 'inside' && {textLabel}} - {progress && labelProgress && progressLabelPosition === 'inside' && {progress}%} + {textLabel && labelText && textLabelPosition === 'inside' && ( + {textLabel} + )} + {progress && labelProgress && progressLabelPosition === 'inside' && ( + {progress}% + )}
From bb3377eb2a1fd017942848e048ceb597961cddad Mon Sep 17 00:00:00 2001 From: revnandi Date: Tue, 31 Jan 2023 19:08:28 +0100 Subject: [PATCH 6/6] add unit tests --- src/lib/components/Progress/Progress.spec.tsx | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/lib/components/Progress/Progress.spec.tsx b/src/lib/components/Progress/Progress.spec.tsx index 7a6d0e4d9..ac375fb90 100644 --- a/src/lib/components/Progress/Progress.spec.tsx +++ b/src/lib/components/Progress/Progress.spec.tsx @@ -29,7 +29,7 @@ describe.concurrent('Components / Progress', () => { expect(progressBar()).toHaveTextContent('Flowbite'); }); - it('should display labels in specified positions', () => { + it('should display test label inside, progress label outside', () => { render( { ); expect(outerLabelContainer()).toBeInTheDocument(); - expect(outerProgressLabel()).toHaveTextContent('45'); + expect(outerProgressLabel()).toHaveTextContent('45%'); expect(innerTextLabel()).toHaveTextContent('Flowbite'); }); + + it('should display text label outside, progress label inside', () => { + render( + , + ); + + 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');