-
Notifications
You must be signed in to change notification settings - Fork 276
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(web-core): circle progress bar component #8402
base: master
Are you sure you want to change the base?
Conversation
2f71351
to
05503e3
Compare
bb7e54b
to
0f0982d
Compare
<style lang="postcss" scoped> | ||
.progress-circle-container { | ||
position: relative; | ||
display: inline-flex; | ||
align-items: center; | ||
justify-content: center; | ||
} | ||
|
||
.progress-circle-background { | ||
stroke: v-bind(backgroundStrokeColor); | ||
stroke-width: v-bind(strokeWidth); | ||
} | ||
|
||
.progress-circle-foreground { | ||
stroke-width: v-bind(strokeWidth); | ||
stroke-linecap: butt; | ||
transition: stroke-dashoffset 0.3s ease; | ||
transform: rotate(-90deg); | ||
transform-origin: center; | ||
} | ||
|
||
.progress-circle-fill { | ||
stroke: v-bind(strokeColor); | ||
stroke-dasharray: v-bind(circumference); | ||
stroke-dashoffset: v-bind(dashOffset); | ||
} | ||
|
||
.progress-circle-overlay { | ||
position: absolute; | ||
top: 50%; | ||
left: 50%; | ||
transform: translate(-50%, -50%); | ||
} | ||
|
||
.progress-circle-text { | ||
color: v-bind(strokeColor); | ||
} | ||
|
||
.progress-circle-icon { | ||
font-size: v-bind(iconSize); | ||
} | ||
</style> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remember to use our guidelines for CSS
@@ -0,0 +1,152 @@ | |||
<!-- v3 --> | |||
<template> | |||
<div class="progress-circle-container"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CSS class should match the component's name and the guidelines
} | ||
|
||
const iconSizeMap = { | ||
'extra-small': '', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can use undefined
here? The same applies on line 70.
large: 16, | ||
} | ||
|
||
const fontClasses = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To keep consistency?
const fontClasses = { | |
const fontClassesMap = { |
accent: ProgressCircleAccent | ||
size: ProgressCircleSize | ||
value: number | ||
maxValue: number |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if maxValue
could be optional, and in this case, compute the completion percentage based on a default value?
I mean, in most cases, we would want to display the completion based on 100%, so maybe this could be the default value?
return circumference.value * (1 - valuePercent.value / 100) | ||
}) | ||
|
||
const percentValue = computed(() => `${valuePercent.value}%`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use n
from useI18n()
, it will handle space between the value and %
, which may differ based on the locale:
const percentValue = computed(() => `${valuePercent.value}%`) | |
const percentValue = computed(() => n(valuePercent.value / 100, 'percent')) |
const backgroundStrokeColor = computed(() => `var(--color-${accent}-background-selected)`) | ||
|
||
const iconAccent = computed(() => | ||
isComplete.value && (accent === 'info' || accent === 'success') ? 'success' : accent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the condition isComplete.value && (accent === 'info' || accent === 'success')
is used twice, maybe it could be refactored into a computed property:
const isCompleteWithSuccess = computed(() => isComplete.value && ['info', 'success'].includes(accent))
/> | ||
</svg> | ||
<div v-if="size !== 'extra-small'" class="progress-circle-overlay"> | ||
<VtsIcon v-if="isComplete" :icon="icon" class="progress-circle-icon" :accent="iconAccent" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<VtsIcon v-if="isComplete" :icon="icon" class="progress-circle-icon" :accent="iconAccent" /> | |
<VtsIcon v-if="isComplete" :icon class="progress-circle-icon" :accent="iconAccent" /> |
v-slot="{ properties }" | ||
:params="[ | ||
prop('value').num().preset(75).required().widget(), | ||
prop('max-value').num().default(100).preset(100).widget(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The prop is required, plus there is no default value (only preset
is enough):
prop('max-value').num().default(100).preset(100).widget(), | |
prop('max-value').num().required().preset(100).widget(), |
const presets = { | ||
'Half of 500': { | ||
props: { | ||
'max-value': 500, | ||
value: 250, | ||
}, | ||
}, | ||
'75% of 300': { | ||
props: { | ||
'max-value': 300, | ||
value: 225, | ||
}, | ||
}, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Be careful to use all the required props (accent
and size
are missing)
Description
Create a circle progress bar component.
4 sizes (extra small, small, medium, large)
4 accents (info, success, warning, medium)
Screenshot
Checklist
Fixes #007
,See xoa-support#42
,See https://...
)Introduced by
CHANGELOG.unreleased.md
Review process
Notes: