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.

Sign up for GitHub

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

Jump to bottom

Add an indeterminate linear progress bar #3429

Merged
merged 45 commits into from
Jul 8, 2024
Merged

Add an indeterminate linear progress bar #3429

merged 45 commits into from
Jul 8, 2024

Conversation

JoanaMMoreira
Copy link
Contributor

@JoanaMMoreira JoanaMMoreira commented May 15, 2024 •
edited by origami-z
Loading

Add indeterminate variant to LinearProgress

#3354

@vercel Vercel
Copy link

vercel bot commented May 15, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
saltdesignsystem ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 8, 2024 2:09pm

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented May 15, 2024 •
edited
Loading

🦋 Changeset detected

Latest commit: 2533ded

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@salt-ds/core Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@JoanaMMoreira JoanaMMoreira marked this pull request as ready for review May 15, 2024 15:43
@JoanaMMoreira JoanaMMoreira changed the title [WIP] Indeterminate linear progress bar Indeterminate linear progress bar May 22, 2024
origami-z
origami-z reviewed May 23, 2024
View reviewed changes
Copy link
Contributor

@origami-z origami-z left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need a site doc example update?

Otherwise looks good

.changeset/four-bugs-reply.md Outdated Show resolved Hide resolved
@JoanaMMoreira
Copy link
Contributor Author

Do we need a site doc example update?

Otherwise looks good

I've updated the original example but I'm not sure if it should be separate? Will need some wording around it if that's the case

@origami-z
Copy link
Contributor

I've updated the original example but I'm not sure if it should be separate? Will need some wording around it if that's the case

I think we need some extra design guidance on when to use and avoid. maybe MUI would have something to get us started

joshwooding
joshwooding reviewed May 23, 2024
View reviewed changes
Copy link
Contributor

@joshwooding joshwooding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a few comments. but we should also show going from an indeterminate to a determinate progress bar.

@JoanaMMoreira
Copy link
Contributor Author

Left a few comments. but we should also show going from an indeterminate to a determinate progress bar.

Do you mean having an example that toggles between those two states?

@joshwooding
Copy link
Contributor

Left a few comments. but we should also show going from an indeterminate to a determinate progress bar.

Do you mean having an example that toggles between those two states?

We need an example that shows a realistic use-case.

joshwooding
joshwooding requested changes May 24, 2024
View reviewed changes
Copy link
Contributor

@joshwooding joshwooding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just marking it as needs changes to make it clear

@JoanaMMoreira
Copy link
Contributor Author

Left a few comments. but we should also show going from an indeterminate to a determinate progress bar.

Do you mean having an example that toggles between those two states?

We need an example that shows a realistic use-case.

@navkaur76 any thoughts on the best example for this?

origami-z
origami-z previously approved these changes Jun 3, 2024
View reviewed changes
JoanaMMoreira and others added 21 commits July 8, 2024 15:03
@JoanaMMoreira @joshwooding
add toast example to site
e5af792
@JoanaMMoreira @joshwooding
add indeterminate bar width var
34bf12f
@JoanaMMoreira @joshwooding
change animation to ease in out
69a9b1d
@origami-z @joshwooding
Remove site example change state
1f7bbd8
@origami-z @joshwooding
Remove variant prop, make it depends on value
5c943a1
@origami-z @joshwooding
Fix typecheck
3b8036a
@origami-z @joshwooding
Indeterminate with buffer value
516763e
@origami-z @joshwooding
Fix indeterminate with buffer
526c94d
@origami-z @joshwooding
Update progress site example
66a3b40
@origami-z @joshwooding
Remove noAnimation
8f0b7c2
@origami-z @joshwooding
Update to use transform
855ee22
@origami-z @joshwooding
Update example and site
d6c3c42
@origami-z @joshwooding
Update changeset
7d90748
@origami-z @joshwooding
Update min/max site heading
ebf3b88
@origami-z @joshwooding
Update buffer doc
0db72ed
@origami-z @joshwooding
Update props jsdoc
0c22b4d
@origami-z @joshwooding
Add back noAnimation css
d1f985a
@origami-z @joshwooding
Apply suggestions from code review
9097191 
Co-authored-by: Josh Wooding <[email protected]>
@navkaur76 @joshwooding
Update examples.mdx
2474bc7 
Updated description of when to use indeterminate linear progress
@origami-z @joshwooding
Update .changeset/four-bugs-reply.md
607c7dc 
Co-authored-by: Josh Wooding <[email protected]>
@origami-z @joshwooding
Prettier
2533ded
@joshwooding joshwooding force-pushed the indeterminate-linear-progress-bar branch from 80f55e2 to 2533ded Compare July 8, 2024 14:03
@joshwooding joshwooding changed the title Indeterminate linear progress bar Add an indeterminate linear progress bar Jul 8, 2024
@origami-z origami-z merged commit 8b43adf into main Jul 8, 2024
15 checks passed
@origami-z origami-z deleted the indeterminate-linear-progress-bar branch July 8, 2024 16:00
mark-tate
mark-tate reviewed Jul 8, 2024
View reviewed changes
packages/core/src/progress/LinearProgress/LinearProgress.tsx
@@ -43,8 +45,8 @@ export const LinearProgress = forwardRef<HTMLDivElement, LinearProgressProps>(
hideLabel = false,
max = 100,
min = 0,
value = 0,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this considered a breaking change ?
Should we leave this as 0 and opt into indeterminate ?
At some point a breaking change release we could make this the default ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mark-tate mark-tate mark-tate left review comments

@joshwooding joshwooding joshwooding approved these changes

@origami-z origami-z origami-z approved these changes

@Fercas123 Fercas123 Fercas123 left review comments

@navkaur76 navkaur76 Awaiting requested review from navkaur76

Assignees

@origami-z origami-z

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@JoanaMMoreira @origami-z @joshwooding @mark-tate @Fercas123 @navkaur76