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

feat(Progress): new component #697

Merged
merged 7 commits into from
Oct 27, 2023
Merged

Conversation

DarkGhostHunter
Copy link
Contributor

@DarkGhostHunter DarkGhostHunter commented Sep 19, 2023

What?

Resolves #691

This is a new component that uses <progress> behind the scenes, with specific pseudo-element fixes for Gecko/Quantum (Firefox) and Webkit/Blink (Chromium, Safari).

Some additional features:

  • Prop parity with <progress> (value, max)
  • Progress (percent) indicator can be enabled with indicator and is shown on top.
  • The max accepts an array of string to use as label below the progress bar.
  • Detects intermediate when value is not issued or is null, showing an animated bar.
  • Supports color styling and sizing.
  • Slot for the indicator
  • Slots for each step

The docs can be better regarding minimum and maximum values for props, or setting the type when there is more than one (like in the case of max, which is [Number, Array].

Also, the ComponentCard should allow to override the <script>, <template>, or both.

@vercel
Copy link

vercel bot commented Sep 19, 2023

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

Name Status Preview Updated (UTC)
ui ✅ Ready (Inspect) Visit Preview Oct 27, 2023 10:57am

@benjamincanac
Copy link
Member

Is this a duplicate of #696? Your branch is not up-to-date with dev.

@DarkGhostHunter
Copy link
Contributor Author

DarkGhostHunter commented Sep 19, 2023

Oh, I see. Gonna fix that.

It's not a duplicate. This uses <progress> and has some new features on top (like steps). I think the team will have to decide on which one to pass, if any.

@Levy-from-Odessa
Copy link
Contributor

Oh, I see. Gonna fix that.

It's not a duplicate. This uses <progress> and has some new features on top (like steps). I think the team will have to decide on which one to pass, if any.

I think your one is quite good. Just add from my MR:
animations,
indicator position,
make it possible to use a slot instead of an indicator,
loading mode (where no value is passed),
and MB min prop...

@DarkGhostHunter , what do you think?

@DarkGhostHunter
Copy link
Contributor Author

Animations

That can be fixed by adding a lost for the bar and let the dev create it's own animation. The problem is that in WebKit/Blink uses before, while Gecko uses --moz-progess-bar. I'll give it a shot to enable that.

Indicator position

That can be fixed with just passing the slot with the current percent.

<template #indicator="{ percent }" />

Indicator slot

As the above.

Loading mode

It's called indeterminate mode from the tag point of view. When it's indeterminate, it shows the bar animation.

Min prop

The <progress> tag has hardcoded 0 as minimum value. It's left to the developer how to adjust its value. May be i give it a shot on a next version.

@DarkGhostHunter
Copy link
Contributor Author

@benjamincanac Waiting for approval.

@DarkGhostHunter
Copy link
Contributor Author

Seems the animation broke on iOS. I'll make some changes.

@DarkGhostHunter
Copy link
Contributor Author

Animations are fixed on Chromium, Firefox and Safari.

That's it for me. Looking up to see it live.

@eduayme
Copy link
Contributor

eduayme commented Sep 20, 2023

Any possibility to add support for inside ranges? Something like min and max?
I would love to be able to use this for price ranges value like this:

Frame 416

@DarkGhostHunter
Copy link
Contributor Author

DarkGhostHunter commented Sep 20, 2023

Any possibility to add support for inside ranges? Something like min and max? I would love to be able to use this for price ranges value like this:

Frame 416

Semantically, that's a job for <meter>, not <progress>.

You gave me an idea for a new component. I hate you.

Copy link
Member

Does this resolves #691?

@DarkGhostHunter
Copy link
Contributor Author

Does this resolves #691?

Yes, check the deployment->edge->meter->indeterminate.

Copy link
Member

Will review asap.

@DarkGhostHunter
Copy link
Contributor Author

Will review asap.

While you're active, check why the hell the dev branch nuxi dev docs gives errors on LandingCard.vue (related to something left from Nuxt Pro UI).

@DarkGhostHunter
Copy link
Contributor Author

@benjamincanac Ready for review.

@DarkGhostHunter DarkGhostHunter mentioned this pull request Oct 18, 2023
8 tasks
@MuhammadM1998
Copy link
Contributor

MuhammadM1998 commented Oct 18, 2023

The percent is not calculated correctly & the indicator overflows if value > max
Screenshot1
Screenshot2

Replacing this line with following should work I think

const percent = computed(() => (Math.max(0, Math.max(props.value, realMax.value)) * 100) / realMax.value)

const percent = computed(() => {
// Always return 0 for negative values
if (props.value < 0)
  return 0
// Always return 100 for values larger than max
else if (props.value > realMax.value)
  return 100
// Otherwise return value/max percentage
else
  return (props.value / realMax.value) * 100
})

Thanks a lot for your work I hope this and meter get merged soon 🙏

@DarkGhostHunter
Copy link
Contributor Author

@MuhammadM1998 Thanks. Got a brain fart

@benjamincanac
Copy link
Member

@DarkGhostHunter Thanks a lot for this awesome component! I've made a few changes if you want to take a look before I merge it 😊

@benjamincanac benjamincanac merged commit 2c5559b into nuxt:dev Oct 27, 2023
2 checks passed
@DarkGhostHunter
Copy link
Contributor Author

DarkGhostHunter commented Oct 27, 2023

Just saw the edits on the sizing. I think it's better.

The only thing I was wondering if the UI tree is too convoluted. I tried my best by using the native tag, but a wrapper was required to make it work, plus a bunch of other properties.

Copy link
Member

You mean the DOM?

@DarkGhostHunter
Copy link
Contributor Author

You mean the DOM?

No, the UI object where the classes are. Tried to slim it down to no avail

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Element] Progress bar component
6 participants