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

[Enhancement] Simplify chart label logic #2085

Closed
MadsBuchmann opened this issue Mar 4, 2022 · 1 comment
Closed

[Enhancement] Simplify chart label logic #2085

MadsBuchmann opened this issue Mar 4, 2022 · 1 comment
Labels
enhancement New feature or request

Comments

@MadsBuchmann
Copy link
Contributor

MadsBuchmann commented Mar 4, 2022

In the current version of Kirby, if a chart is created where an empty array is passed to the labels input property, we will auto generate blank labels to apply to the x-axis. Otherwise the chart will not render:

<kirby-chart [data]="[1,2,3]" [labels]="[]"></kirby-chart>

In #2082 it was however discovered that there exist cases where we want to be able to pass an empty array of labels to the chart. For example when utilizingtimeScale via customOptions. Here we want the default behaviour of chart.js - generating blank labels will result in them being appended to the x-axis, causing the chart to render incorrectly.

Logic was introduced in #2082 to make it possible to use a timeScale with the stock chart type. But to simplify this logic, i think we should simply always pass labels directly to chart.js if anything is given to the labels input property.

Only if nothing is passed in here should we have some default behaviour.

Pseudo-code:

if labels are given: return labels 
else if chart type is stock: return defaultStockChartLabels
else: return blank labels

This would be a breaking change as a chart now works with an empty array; it wouldn't after this change.

@MadsBuchmann MadsBuchmann added enhancement New feature or request NOT Tech refined Needs Tech kickoff - solution outlined and agreed NOT Prioritized Issue not yet prioritized and added to a Milestone 👶🏻 New For new issues before prioritisation and refinement labels Mar 4, 2022
@alxzak alxzak removed the 👶🏻 New For new issues before prioritisation and refinement label Mar 4, 2022
@MadsBuchmann MadsBuchmann changed the title [Enhancement] PLACEHOLDER [Enhancement] Simplify chart label logic Mar 4, 2022
@alxzak alxzak added this to the M11 ( 27/1 - 6/4 ) milestone Mar 4, 2022
@alxzak alxzak added this to Kirby Mar 4, 2022
@MadsBuchmann MadsBuchmann removed NOT Tech refined Needs Tech kickoff - solution outlined and agreed NOT Prioritized Issue not yet prioritized and added to a Milestone labels Mar 7, 2022
@RasmusKjeldgaard RasmusKjeldgaard moved this to 📙 Backlog in Kirby Mar 15, 2022
@MadsBuchmann
Copy link
Contributor Author

Should be solved as part of #2108

Repository owner moved this from 📙 Backlog to ✅ Done in Kirby Mar 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

No branches or pull requests

2 participants