Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: add display remaining feat for ProgressBar #700

Merged
merged 7 commits into from
Nov 5, 2021

Conversation

subirjolly
Copy link
Contributor

@subirjolly subirjolly commented Nov 5, 2021

Closes #

Changes

Added displayRemaining boolean flag which allows you to view remaining value instead of used value.
Also add prefix and suffix to value/max labels.

Screenshots

If displayRemaining boolean value is passed as true then following is the affect on the ProgressBar. Also added prefix and suffix to value/max labels.

BEFORE
image

AFTER
image

Checklist

Check all that apply

  • Updated documentation to reflect changes
  • Added entry to top of Changelog with link to PR (not issue)
  • Tests pass
  • Peer reviewed and approved
  • Signed CLA (if not already signed)

@subirjolly subirjolly marked this pull request as draft November 5, 2021 19:42
@subirjolly subirjolly marked this pull request as ready for review November 5, 2021 19:46
@daviddkkim
Copy link
Contributor

daviddkkim commented Nov 5, 2021

@subirjolly in the screenshot, is the reason why the progress bar doesn't fill up the height of the container because of CSS you passed in on the uI repo or is it like that in clockface?

if the former, it might make sense to create different sizing for the progressbar in clockface.

Also, I'm not sure if this pattern will work smoothly. There should be some indication that one is talking about remaining and the other is talking about used. Two UI that communicate different things look entirely the same

for example, When communicating "9" is remaining, does it matter that the total was 30? or does it only really matter that you have 9 remaining? So maybe we could make it 9 Remaining rather than 9/30 ?

@subirjolly
Copy link
Contributor Author

subirjolly commented Nov 5, 2021

@subirjolly in the screenshot, is the reason why the progress bar doesn't fill up the height of the container because of CSS you passed in on the uI repo or is it like that in clockface?

It doesn't seem to be because of my CSS as I have disabled my CSS for this component all together.

if the former, it might make sense to create different sizing for the progressbar in clockface.

Also, I'm not sure if this pattern will work smoothly. There should be some indication that one is talking about remaining and the other is talking about used. Two UI that communicate different things look entirely the same

Makes sense and I am about to add additional label for the values.

for example, When communicating "9" is remaining, does it matter that the total was 30? or does it only really matter that you have 9 remaining? So maybe we could make it 9 Remaining rather than 9/30 ?

I don't think this should matter, for this change. According to the mockups I am working with, we do need the latter part as well.

Copy link
Contributor

@daviddkkim daviddkkim left a comment

Choose a reason for hiding this comment

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

Subir and I chat about what he could do.
Value text and max text

approving in advance since i will be offline

@subirjolly subirjolly requested a review from daviddkkim November 5, 2021 21:20
Copy link
Contributor

@daviddkkim daviddkkim left a comment

Choose a reason for hiding this comment

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

lgtm

@subirjolly subirjolly merged commit 1dd0704 into master Nov 5, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants