-
Notifications
You must be signed in to change notification settings - Fork 23
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
Allow chart labels to be provided together datasets #2157
Allow chart labels to be provided together datasets #2157
Conversation
libs/designsystem/src/lib/components/chart/chart-js/chart-js.service.ts
Outdated
Show resolved
Hide resolved
libs/designsystem/src/lib/components/chart/chart-js/chart-js.service.ts
Outdated
Show resolved
Hide resolved
return datasets.some(({ data }) => { | ||
if (data === undefined) return false; | ||
return data.some((datapoint) => typeof datapoint[indexAxis] === 'string'); | ||
}); |
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.
return datasets.some(
({ data }) =>
data !== undefined &&
data.some(
(datapoint) => typeof datapoint === 'object' && typeof datapoint[indexAxis] === 'string'
)
);
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.
Excellent test coverage.
Added a few comments aiming at improving the understanding of the intention behind the somewhat complex label handling in the component.
libs/designsystem/src/lib/components/chart/chart-js/chart-js.service.spec.ts
Show resolved
Hide resolved
* ♻️ Redo label logic * 🐛 Have labels be used before dataset strings * ✅ Add tests for new label logic * ♻️ Combine datasetsHaveLabels & labelsToApply * 🎨 Remove unused import, prettier changes * 🏷️ Introduce ChartLabels type for better readability * ♻️ Refactor for better readability * ♻️ Attempt to improve readability of getLabelsToApply * 🤡 Use 'ChartLabel' type in mock * 🏷️ Add type to 'indexAxis' argument * 🚨 Fix line endings
* Calculate UTC offset based on correct date (#2115) * 🚧 UTC time experiment * ⚗️ Do not use subtractTimezoneOffset() * 🐛 Remove unneccessary timezone offset * 🐛 Add back offset to preserve existing functionality It seems that the original intent was to simply format a local time as its UTC equivalent, and not adjust it to any specific UTC zone. This is done by consumers, so something like 02:00:00 GMT+0200 becomes 00:00:00 GMT on their end. * ⏪ Revert changes to showcase component Co-authored-by: Jesper Kaltoft <git@jesperkaltoft.dk> * Upgrade prettier and stylelint (#2146) * ⬆️ Upgrade prettier * 💄 Lint and prettify cookbook styles * 🎨 Lint and prettify cookbook ts files * 🎨 Lint and prettify cookbook templates * 🔧 Update prettier and stylelint config * 🎨 Lint and prettify misc project files * 🎨 Lint and prettify designsystem styles * 🤡 Use same line endings across mocks * 🎨 Lint and prettify designsystem spec files * 🎨 Lint and prettify designsystem ts files * 💄 Rename css-class in template * ✅ Fix tab test according to changed css * 🙈 Update ignore files with newline at EOF * ♻️ Increase readability of span * 🔧 Prettify .stylelintrc (whoa, meta!) * ➖ Remove unused depencency * 💄 Replace duplicate ::after with ::before for print * ⏪ Revert unintentional prettier change * 🔥 Remove vendor prefix This is handled by Angular via auto-prefixer. * ⏪ Revert unintentional change to line clamp styles * Allow non-enum Input in components: template literal type solution (#2099) * Enable non-enum input * Update button.component.ts * Use values when enum is mapping * allow use of string values for enum input props * 🏷️ Allow non-enum input types * 🏷️ Update mocks to use template literal types Co-authored-by: Morten Bjerg Gregersen <mgr@bankdata.dk> * Add unit back on item safe area custom property (#2160) * 💄 Add unit back on item safe area custom property * 💄 Make stylelint ignore 0px for custom properties * 💡 Remove stylelint ignores * Set fallback value for --padding-top/bottom on item (#2118) * 🐛 Set padding if custom css prop is not set * 💄 Default to initial value * 🐛 Revert to specific fallback value for clarity * 🍱 Replace more icon file with the one from sketch (#2176) * Allow chart labels to be provided together datasets (#2157) * ♻️ Redo label logic * 🐛 Have labels be used before dataset strings * ✅ Add tests for new label logic * ♻️ Combine datasetsHaveLabels & labelsToApply * 🎨 Remove unused import, prettier changes * 🏷️ Introduce ChartLabels type for better readability * ♻️ Refactor for better readability * ♻️ Attempt to improve readability of getLabelsToApply * 🤡 Use 'ChartLabel' type in mock * 🏷️ Add type to 'indexAxis' argument * 🚨 Fix line endings * Flag: New danger color and heavier font weight (#2156) * 💄 Introduce color variables (tokens) * 💄 Override danger color in flag component * 💄 Use font-weight 500 for text in flag component * ✅ Add font-weight test * ✅ Change test for flag danger theme * Add back CSS Custom Property px unit (#2183) * ⏪ Revert removal of px unit for custom props The CSS Custom Properties that had px before, but was removed in 6ebc263 now have the unit back, as it caused some issues for components where the prop is used in calc() etc. * ✅ Add px value to test * Release version 5.3 Co-authored-by: Jesper Kaltoft <git@jesperkaltoft.dk> Co-authored-by: Mads Buchmann Frederiksen <mbf@prochimp.dk> Co-authored-by: Morten Bjerg Gregersen <mgr@bankdata.dk>
Which issue does this PR close?
This PR closes #2108
This PR also solves a bug originating from #kirby-guild which resulted in some stock charts displaying with 'invalid date' as labels.
What is the new behavior?
The logic has been simplified to no longer care about type and it has been extended to allow datasets with labels.
Does this PR introduce a breaking change?
Are there any additional context?
Checklist:
The following tasks should be carried out in sequence in order to follow the process of contributing correctly.
Reminders
Make sure you have updated the cookbook with examples and showcases (for bug fixes, enhancements & new components).Review
Request that the changes are UX reviewed (only necessary if your PR introduces visual changes)When the pull request has been approved it will be automatically merged to
stable
via automerge.