refactor Progress to remove label and description slots (#25067)

* refactor Progress to remove label and description slots * api and snapshot update * Apply suggestions from code review Co-authored-by: Makoto Morimoto <[email protected]> * Apply suggestions from code review Co-authored-by: Ben Howell <[email protected]> * api and stories update * Update packages/react-components/react-progress/src/stories/Progress/ProgressIndeterminate.stories.tsx Co-authored-by: Ben Howell <[email protected]> * update stories * Update packages/react-components/react-progress/src/components/Progress/Progress.types.ts Co-authored-by: Ben Howell <[email protected]> Co-authored-by: Makoto Morimoto <[email protected]> Co-authored-by: Ben Howell <[email protected]>
microsoft · Oct 5, 2022 · 5a3ac20 · 5a3ac20
1 parent 1c8b241
commit 5a3ac20
Show file tree

Hide file tree

Showing 12 changed files with 70 additions and 224 deletions.
diff --git a/packages/react-components/react-progress/etc/react-progress.api.md b/packages/react-components/react-progress/etc/react-progress.api.md
@@ -19,22 +19,19 @@ export const progressClassNames: SlotClassNames<ProgressSlots>;
 
 // @public
 export type ProgressProps = Omit<ComponentProps<ProgressSlots>, 'size'> & {
-    indeterminate?: boolean;
-    percentComplete?: number;
+    value?: number;
+    max?: number;
     thickness?: 'medium' | 'large';
 };
 
 // @public (undocumented)
 export type ProgressSlots = {
     root: NonNullable<Slot<'div'>>;
-    label?: Slot<'span'>;
     bar?: NonNullable<Slot<'div'>>;
-    track?: NonNullable<Slot<'div'>>;
-    description?: Slot<'span'>;
 };
 
 // @public
-export type ProgressState = ComponentState<ProgressSlots> & Required<Pick<ProgressProps, 'indeterminate' | 'percentComplete' | 'thickness'>>;
+export type ProgressState = ComponentState<ProgressSlots> & Required<Pick<ProgressProps, 'max' | 'thickness'>> & Pick<ProgressProps, 'value'>;
 
 // @public
 export const renderProgress_unstable: (state: ProgressState) => JSX.Element;

diff --git a/packages/react-components/react-progress/src/components/Progress/Progress.types.ts b/packages/react-components/react-progress/src/components/Progress/Progress.types.ts
@@ -2,45 +2,32 @@ import type { ComponentProps, ComponentState, Slot } from '@fluentui/react-utili
 
 export type ProgressSlots = {
   /**
-   * The root of the Progress
-   * The root slot receives the `className` and `style` specified directly on the `<Progress>`.
+   * The track behind the progress bar
    */
   root: NonNullable<Slot<'div'>>;
   /**
-   * The title of the Progress.
-   * The label slot receives the styling related to the title associated with the Progress.
-   */
-  label?: Slot<'span'>;
-  /**
-   * The animated slot of the Progress
-   * The bar slot receives the styling related to the loading bar associated with the Progress
+   * The filled portion of the progress bar. Animated in the indeterminate state, when no value is provided.
    */
   bar?: NonNullable<Slot<'div'>>;
-  /**
-   * The track slot of the Progress
-   * The track slot receives the styling related to the loading bar track associated with the Progress
-   */
-  track?: NonNullable<Slot<'div'>>;
-  /**
-   * The description slot of the Progress
-   * The description slot receives the styling related to the description associated with the Progress
-   */
-  description?: Slot<'span'>;
 };
 
 /**
  * Progress Props
  */
 export type ProgressProps = Omit<ComponentProps<ProgressSlots>, 'size'> & {
   /**
-   * Prop to set whether the Progress is determinate or indeterminate
-   * @default false
+   * A decimal number between `0` and `1` (or between `0` and `max` if given),
+   * which specifies how much of the task has been completed. 
+   *
+   * If `undefined` (default), the Progress will display an **indeterminate** state.
    */
-  indeterminate?: boolean;
+  value?: number;
   /**
-   * Percentage of the operation's completeness, numerically between 0 and 100.
+   * The maximum value, which indicates the task is complete.
+   * The progress bar will be full when `value` equals `max`.
+   * @default 1
    */
-  percentComplete?: number;
+  max?: number;
   /**
    * The thickness of the Progress bar
    * @default 'medium'
@@ -51,5 +38,4 @@ export type ProgressProps = Omit<ComponentProps<ProgressSlots>, 'size'> & {
 /**
  * State used in rendering Progress
  */
-export type ProgressState = ComponentState<ProgressSlots> &
-  Required<Pick<ProgressProps, 'indeterminate' | 'percentComplete' | 'thickness'>>;
+export type ProgressState = ComponentState<ProgressSlots> & Required<Pick<ProgressProps, 'max' | 'thickness'>> & Pick<ProgressProps, 'value'>;
diff --git a/...ct-components/react-progress/src/components/Progress/__snapshots__/Progress.test.tsx.snap b/...ct-components/react-progress/src/components/Progress/__snapshots__/Progress.test.tsx.snap
@@ -7,14 +7,7 @@ exports[`Progress renders a default state 1`] = `
     role="progressbar"
   >
     <div
-      class="fui-Progress__track"
-    />
-    <div
-      aria-valuemax="100"
-      aria-valuemin="0"
-      aria-valuenow="0"
       class="fui-Progress__bar"
-      style="--fui-Progress--percentage: 0%;"
     />
   </div>
 </div>

diff --git a/packages/react-components/react-progress/src/components/Progress/renderProgress.tsx b/packages/react-components/react-progress/src/components/Progress/renderProgress.tsx
@@ -7,12 +7,5 @@ import type { ProgressState, ProgressSlots } from './Progress.types';
  */
 export const renderProgress_unstable = (state: ProgressState) => {
   const { slots, slotProps } = getSlots<ProgressSlots>(state);
-  return (
-    <slots.root {...slotProps.root}>
-      {slots.label && <slots.label {...slotProps.label} />}
-      {slots.track && <slots.track {...slotProps.track} />}
-      {slots.bar && <slots.bar {...slotProps.bar} />}
-      {slots.description && <slots.description {...slotProps.description} />}
-    </slots.root>
-  );
+  return <slots.root {...slotProps.root}>{slots.bar && <slots.bar {...slotProps.bar} />}</slots.root>;
 };
diff --git a/packages/react-components/react-progress/src/components/Progress/useProgress.tsx b/packages/react-components/react-progress/src/components/Progress/useProgress.tsx
@@ -1,5 +1,5 @@
 import * as React from 'react';
-import { getNativeElementProps, resolveShorthand, useId } from '@fluentui/react-utilities';
+import { getNativeElementProps, resolveShorthand } from '@fluentui/react-utilities';
 import type { ProgressProps, ProgressState } from './Progress.types';
 
 /**
@@ -13,56 +13,29 @@ import type { ProgressProps, ProgressState } from './Progress.types';
  */
 export const useProgress_unstable = (props: ProgressProps, ref: React.Ref<HTMLElement>): ProgressState => {
   // Props
-  const { thickness = 'medium', indeterminate = false, percentComplete = 0 } = props;
-  const baseId = useId('progress-');
+  const { thickness = 'medium', value, max = 1.0 } = props;
 
   const root = getNativeElementProps('div', { ref, role: 'progressbar', ...props });
 
-  const label = resolveShorthand(props.label, {
-    defaultProps: {
-      id: baseId + '__label',
-    },
-  });
-
-  const description = resolveShorthand(props.description, {
-    defaultProps: {
-      id: baseId + '__description',
-    },
-  });
-
   const bar = resolveShorthand(props.bar, {
     required: true,
     defaultProps: {
-      'aria-valuemin': indeterminate ? undefined : 0,
-      'aria-valuemax': indeterminate ? undefined : 100,
-      'aria-valuenow': indeterminate ? undefined : Math.floor(percentComplete),
+      'aria-valuemin': value ? 0 : undefined,
+      'aria-valuemax': value ? max : undefined,
+      'aria-valuenow': value,
     },
   });
 
-  const track = resolveShorthand(props.track, {
-    required: true,
-  });
-
-  if (label && !root['aria-label'] && !root['aria-labelledby']) {
-    root['aria-labelledby'] = label.id;
-  }
-
   const state: ProgressState = {
-    indeterminate,
-    percentComplete,
+    max,
     thickness,
+    value,
     components: {
       root: 'div',
       bar: 'div',
-      track: 'div',
-      label: 'span',
-      description: 'span',
     },
     root,
     bar,
-    track,
-    label,
-    description,
   };
 
   return state;

diff --git a/packages/react-components/react-progress/src/components/Progress/useProgressStyles.ts b/packages/react-components/react-progress/src/components/Progress/useProgressStyles.ts
@@ -1,26 +1,18 @@
 import { makeStyles, mergeClasses, shorthands } from '@griffel/react';
-import { tokens, typographyStyles } from '@fluentui/react-theme';
+import { tokens } from '@fluentui/react-theme';
 import { useFluent_unstable as useFluent } from '@fluentui/react-shared-contexts';
 import type { ProgressState, ProgressSlots } from './Progress.types';
 import type { SlotClassNames } from '@fluentui/react-utilities';
 
 export const progressClassNames: SlotClassNames<ProgressSlots> = {
   root: 'fui-Progress',
   bar: 'fui-Progress__bar',
-  track: 'fui-Progress__track',
-  label: 'fui-Progress__label',
-  description: 'fui-Progress__description',
 };
 
 // If the percentComplete is near 0, don't animate it.
 // This prevents animations on reset to 0 scenarios.
 const ZERO_THRESHOLD = 0.01;
 
-// Internal CSS vars
-export const progressCssVars = {
-  percentageCssVar: '--fui-Progress--percentage',
-};
-
 const barThicknessValues = {
   medium: '2px',
   large: '4px',
@@ -40,31 +32,19 @@ const indeterminateProgress = {
  */
 const useRootStyles = makeStyles({
   root: {
-    display: 'grid',
-    rowGap: '8px',
+    display: 'block',
+    backgroundColor: tokens.colorNeutralBackground6,
     ...shorthands.overflow('hidden'),
-  },
-});
 
-/**
- * Styles for the title
- */
-const useLabelStyles = makeStyles({
-  base: {
-    gridRowStart: '1',
-    ...typographyStyles.body1,
-    color: tokens.colorNeutralForeground1,
+    '@media screen and (forced-colors: active)': {
+      ...shorthands.borderBottom('1px', 'solid', 'CanvasText'),
+    },
   },
-});
-
-/**
- * Styles for the description
- */
-const useDescriptionStyles = makeStyles({
-  base: {
-    gridRowStart: '3',
-    ...typographyStyles.caption1,
-    color: tokens.colorNeutralForeground2,
+  medium: {
+    height: barThicknessValues.medium,
+  },
+  large: {
+    height: barThicknessValues.large,
   },
 });
 
@@ -73,8 +53,6 @@ const useDescriptionStyles = makeStyles({
  */
 const useBarStyles = makeStyles({
   base: {
-    gridColumnStart: '1',
-    gridRowStart: '2',
     backgroundColor: tokens.colorCompoundBrandBackground,
 
     '@media screen and (forced-colors: active)': {
@@ -87,9 +65,6 @@ const useBarStyles = makeStyles({
   large: {
     height: barThicknessValues.large,
   },
-  determinate: {
-    width: `var(${progressCssVars.percentageCssVar})`,
-  },
   nonZeroDeterminate: {
     transitionProperty: 'width',
     transitionDuration: '0.3s',
@@ -114,75 +89,37 @@ const useBarStyles = makeStyles({
   },
 });
 
-const useTrackStyles = makeStyles({
-  base: {
-    gridRowStart: '2',
-    gridColumnStart: '1',
-    backgroundColor: tokens.colorNeutralBackground6,
-
-    '@media screen and (forced-colors: active)': {
-      ...shorthands.borderBottom('1px', 'solid', 'CanvasText'),
-    },
-  },
-  medium: {
-    height: barThicknessValues.medium,
-  },
-  large: {
-    height: barThicknessValues.large,
-  },
-});
-
 /**
  * Apply styling to the Progress slots based on the state
  */
 export const useProgressStyles_unstable = (state: ProgressState): ProgressState => {
-  const { indeterminate, thickness, percentComplete } = state;
+  const { max, thickness, value } = state;
   const rootStyles = useRootStyles();
   const barStyles = useBarStyles();
-  const trackStyles = useTrackStyles();
-  const labelStyles = useLabelStyles();
-  const descriptionStyles = useDescriptionStyles();
   const { dir } = useFluent();
 
-  state.root.className = mergeClasses(progressClassNames.root, rootStyles.root, state.root.className);
+  state.root.className = mergeClasses(
+    progressClassNames.root,
+    rootStyles.root,
+    rootStyles[thickness],
+    state.root.className,
+  );
 
   if (state.bar) {
     state.bar.className = mergeClasses(
       progressClassNames.bar,
       barStyles.base,
-      indeterminate && barStyles.indeterminate,
-      indeterminate && dir === 'rtl' && barStyles.rtl,
+      value === undefined && barStyles.indeterminate,
+      value === undefined && dir === 'rtl' && barStyles.rtl,
       barStyles[thickness],
-      !indeterminate && barStyles.determinate,
-      !indeterminate && percentComplete > ZERO_THRESHOLD && barStyles.nonZeroDeterminate,
+      value !== undefined && value > ZERO_THRESHOLD && barStyles.nonZeroDeterminate,
       state.bar.className,
     );
   }
 
-  if (state.track) {
-    state.track.className = mergeClasses(
-      progressClassNames.track,
-      trackStyles.base,
-      trackStyles[thickness],
-      state.track.className,
-    );
-  }
-
-  if (state.label) {
-    state.label.className = mergeClasses(progressClassNames.label, labelStyles.base, state.label.className);
-  }
-
-  if (state.description) {
-    state.description.className = mergeClasses(
-      progressClassNames.description,
-      descriptionStyles.base,
-      state.description.className,
-    );
-  }
-
-  if (state.bar && !indeterminate) {
+  if (state.bar && value !== undefined) {
     state.bar.style = {
-      [progressCssVars.percentageCssVar]: Math.min(100, Math.max(0, percentComplete)) + '%',
+      width: Math.min(100, Math.max(0, (value / max) * 100)) + '%',
       ...state.bar.style,
     };
   }