-
Notifications
You must be signed in to change notification settings - Fork 262
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(xo-web): add donut chart with legends component #7778
Conversation
P4l0m4
commented
Jun 26, 2024
•
edited
Loading
edited
791e649
to
23a49ef
Compare
export type DonutColor = 'success' | 'warning' | 'error' | 'unknown' | ||
export type DonutSegment = { | ||
value: number | ||
color: DonutColor | ||
} |
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.
DonutSegmentColor
would probably be a more suitable name.
export type DonutColor = 'success' | 'warning' | 'error' | 'unknown' | |
export type DonutSegment = { | |
value: number | |
color: DonutColor | |
} | |
export type DonutSegmentColor = 'success' | 'warning' | 'error' | 'unknown' | |
export type DonutSegment = { | |
value: number | |
color: DonutSegmentColor | |
} |
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 name the file legend.type.ts
label: string | ||
value: number | ||
unit?: string | ||
color: LegendColor |
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.
DonutSegmentColor
being more restrictive than LegendColor
, it would therefore make more sense to use it.
color: LegendColor | |
color: DonutSegmentColor |
const donutSegments = computed(() => { | ||
if (props?.segments.length === 0) { | ||
return [] | ||
} | ||
return props.segments | ||
.filter(segment => segment.color !== 'dark-blue') | ||
.map(segment => { | ||
let color | ||
if (segment.color === 'disabled' || segment.color === 'default') { | ||
color = 'unknown' | ||
} else { | ||
color = segment.color | ||
} | ||
|
||
return { | ||
value: segment.value, | ||
color, | ||
} | ||
}) as DonutSegment[] | ||
}) |
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.
props
always exists, the?
is useless.- No need to check for
segments.length
. Ifprops.segments
is empty, then.filter.map
will return an empty array anyway. - It makes no sense to accept
dark-blue
as segment color if these segments are then completely excluded from the result.
const donutSegments = computed(() => { | |
if (props?.segments.length === 0) { | |
return [] | |
} | |
return props.segments | |
.filter(segment => segment.color !== 'dark-blue') | |
.map(segment => { | |
let color | |
if (segment.color === 'disabled' || segment.color === 'default') { | |
color = 'unknown' | |
} else { | |
color = segment.color | |
} | |
return { | |
value: segment.value, | |
color, | |
} | |
}) as DonutSegment[] | |
}) | |
const donutSegments = computed(() => | |
props.segments.map(segment => ({ | |
value: segment.value, | |
color: segment.color === 'unknown' ? 'disabled' : segment.color, | |
} as DonutSegment)) | |
) |
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 think nothing prevents this component from being moved to Web Core.
4dd7d0f
to
2cbb479
Compare
2cbb479
to
5966365
Compare
ff59dd5
to
c58fb00
Compare
Superseded with #7829 |