[Progress] New `Progress` components #470

mj12albert · 2024-06-28T06:13:43Z

Closes #218

Demo: https://deploy-preview-470--base-ui.netlify.app/base-ui/react-progress/

I have followed (at least) the PR section of the contributing guide.

mui-bot · 2024-06-28T06:19:36Z

Netlify deploy preview

https://deploy-preview-470--base-ui.netlify.app/

Generated by 🚫 dangerJS against 752a574

mj12albert · 2024-07-09T08:56:09Z

@colmtuite The implementation is complete, what do you think about the demos?
I will add the Tailwind/css variants to the intro demo if you're OK with it like this, not sure if you want to animate it or something like that.

michaldudak · 2024-07-19T09:13:50Z

docs/data/base/components/progress/UnstyledProgressIntroduction.tsx

+export function Styles() {
+  const isDarkMode = useIsDarkMode();
+  return (
+    <style suppressHydrationWarning>{`


What this suppression for? If there are differences due to encoding of some characters, try <style dangerouslySetInnerHTML={{ __html: '' }} />

It was for some encoding issue, but I've fixed the root cause now 😅

michaldudak · 2024-07-19T09:15:00Z

packages/mui-base/src/Progress/Root/ProgressRoot.types.ts

+   * The direction that progress bars fill in
+   * @default 'ltr'
+   */
+  direction?: ProgressDirection;


We use dir elsewhere.

The prop is 'direction' ~ e.g. Tabs: https://master--base-ui.netlify.app/base-ui/react-tabs/components-api/#tabs-root-prop-direction

'dir' is the attribute set on the elements via 'direction' 😬

I must have been hallucinating, then, sorry! :)

michaldudak · 2024-07-19T09:16:01Z

packages/mui-base/src/Progress/Root/ProgressContext.tsx

+/**
+ * @ignore - internal component.
+ */
+export const ProgressContext = React.createContext<ProgressContextValue | undefined>(undefined);


It's possible to use the same name for the const and its type:

Suggested change

export const ProgressContext = React.createContext<ProgressContextValue | undefined>(undefined);

export const ProgressContext = React.createContext<ProgressContext | undefined>(undefined);

@michaldudak How can this work? 🤔 I get an error...
All our other components have XXXContextValue as the type name, e.g. export const PopoverContext = React.createContext<PopoverRootContextValue | null>(null)

OK, let's leave it as it is. I'm trying out a different approach in #468. If we decide to adopt it, I'll change all the occurences.

michaldudak · 2024-07-19T09:19:35Z

packages/mui-base/src/Progress/Root/useProgressRoot.ts

+
+  let state: ProgressStatus = 'indeterminate';
+  if (Number.isFinite(value)) {
+    state = value === max ? 'complete' : 'loading';


I'm not sure about "loading". The progress bar doesn't have to show the loading process. Perhaps 'in-progress' | 'complete'? (plus | 'not-started' for completeness)

Good idea renaming 'loading' to 'in-progress', it's closer to the component's name and a bit more flexible (e.g. it could represent "uploading")
But for 'not-started', I think we can't really differentiate that with 'in-progress' - 0% could mean a process has started, but no progress has been made yet (or so little that it rounds down to 0%)

IMO you could make the same argument for complete - it could be 99.9% rounded up to 100%. But I don't insist. Probably best to see with @colmtuite if this third state could be useful for styling.

We have decided on: 'indeterminate' | 'progressing' | 'complete'

michaldudak

LGTM!

…ponent

colmtuite · 2024-07-29T08:57:05Z

@mj12albert What's the purpose of having 3 parts to this component, rather than just Root and Indicator?

mj12albert · 2024-07-29T10:01:28Z

@mj12albert What's the purpose of having 3 parts to this component, rather than just Root and Indicator?

@colmtuite Though this isn't a form control, it's still label-able and I thought its more consistent to recommend placing the label inside the root like other components
With a structure like that, it's inconvenient to style the root to look like the track, so the Track subcomponent is really to avoid having to use a pseudoelement like for slider:

<Progress.Root>
  <Label>Label text</Label>
  <Progress.Indicator />
</Progress.Root>

What do you think?

…ponent

mj12albert added new feature New feature or request component: progress This is the name of the generic UI component, not the React module! labels Jun 28, 2024

mj12albert force-pushed the feat/progress-component branch from 9a8691b to 8426306 Compare June 28, 2024 06:16

mj12albert force-pushed the feat/progress-component branch 2 times, most recently from 18f24f1 to 4a3e1a1 Compare July 1, 2024 09:21

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 2, 2024

mj12albert force-pushed the feat/progress-component branch from 4a3e1a1 to 695fad0 Compare July 4, 2024 05:23

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 4, 2024

mj12albert force-pushed the feat/progress-component branch 13 times, most recently from 9186d7e to 7fef12c Compare July 9, 2024 07:02

mj12albert marked this pull request as ready for review July 9, 2024 08:56

mj12albert requested review from atomiks, colmtuite and michaldudak as code owners July 9, 2024 08:56

mj12albert force-pushed the feat/progress-component branch from 7fef12c to f07c004 Compare July 9, 2024 09:21

michaldudak reviewed Jul 19, 2024

View reviewed changes

mj12albert force-pushed the feat/progress-component branch from f07c004 to 6ef9e31 Compare July 22, 2024 06:47

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 23, 2024

mj12albert force-pushed the feat/progress-component branch from 6ef9e31 to fca034d Compare July 25, 2024 06:04

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 25, 2024

mj12albert force-pushed the feat/progress-component branch 2 times, most recently from 10021b0 to 60c14c1 Compare July 26, 2024 07:46

Add Progress hooks and components

bda349d

mj12albert force-pushed the feat/progress-component branch from 60c14c1 to bda349d Compare July 26, 2024 08:11

michaldudak approved these changes Jul 29, 2024

View reviewed changes

Merge remote-tracking branch 'upstream/master' into feat/progress-com…

8bccd11

…ponent

colmtuite approved these changes Jul 29, 2024

View reviewed changes

Merge remote-tracking branch 'upstream/master' into feat/progress-com…

752a574

…ponent

mj12albert merged commit 0e60b91 into mui:master Jul 30, 2024
18 checks passed

mj12albert deleted the feat/progress-component branch July 30, 2024 14:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Progress] New `Progress` components #470

[Progress] New `Progress` components #470

mj12albert commented Jun 28, 2024 •

edited

Loading

mui-bot commented Jun 28, 2024 •

edited

Loading

mj12albert commented Jul 9, 2024

michaldudak Jul 19, 2024

mj12albert Jul 22, 2024

michaldudak Jul 19, 2024

mj12albert Jul 22, 2024

michaldudak Jul 22, 2024

michaldudak Jul 19, 2024

mj12albert Jul 22, 2024

michaldudak Jul 22, 2024

michaldudak Jul 19, 2024

mj12albert Jul 22, 2024 •

edited

Loading

michaldudak Jul 22, 2024

mj12albert Jul 25, 2024

michaldudak left a comment

colmtuite commented Jul 29, 2024

mj12albert commented Jul 29, 2024

	export const ProgressContext = React.createContext<ProgressContextValue \| undefined>(undefined);
	export const ProgressContext = React.createContext<ProgressContext \| undefined>(undefined);

[Progress] New Progress components #470

[Progress] New Progress components #470

Conversation

mj12albert commented Jun 28, 2024 • edited Loading

mui-bot commented Jun 28, 2024 • edited Loading

Netlify deploy preview

mj12albert commented Jul 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mj12albert Jul 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaldudak left a comment

Choose a reason for hiding this comment

colmtuite commented Jul 29, 2024

mj12albert commented Jul 29, 2024

[Progress] New `Progress` components #470

[Progress] New `Progress` components #470

mj12albert commented Jun 28, 2024 •

edited

Loading

mui-bot commented Jun 28, 2024 •

edited

Loading

mj12albert Jul 22, 2024 •

edited

Loading