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

feat: add title to BarChartRodStackItem #1372

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

ImagineBoom
Copy link

Closes #1371

@codecov
Copy link

codecov bot commented Jul 4, 2023

Codecov Report

Merging #1372 (cdbeb7c) into master (478e5e6) will decrease coverage by 0.01%.
The diff coverage is n/a.

❗ Current head cdbeb7c differs from pull request most recent head 7aaaac3. Consider uploading reports for the commit 7aaaac3 to get more accurate results

@@            Coverage Diff             @@
##           master    #1372      +/-   ##
==========================================
- Coverage   86.09%   86.08%   -0.01%     
==========================================
  Files          45       45              
  Lines        2978     2976       -2     
==========================================
- Hits         2564     2562       -2     
  Misses        414      414              

see 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@imaNNeo
Copy link
Owner

imaNNeo commented Jul 4, 2023

It would be better if we keep a generic type of data (not only strings). For example, we can keep a dynamic type which could be anything

@ImagineBoom
Copy link
Author

ImagineBoom commented Jul 5, 2023

It would be better if we keep a generic type of data (not only strings). For example, we can keep a dynamic type which could be anything

@imaNNeo Thank you😀, I have made the changes.

@imaNNeo
Copy link
Owner

imaNNeo commented Jul 31, 2023

title is not a good name anymore as it is a dynamic field.
I suggest renaming it to something like payload or data.
Also, you need to rebase your branch and write some unit tests for it (read our contributing guidelines carefully).

]);

/// Renders a Stacked Chart section from [title]
Copy link
Owner

Choose a reason for hiding this comment

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

Also please update this comment. It seems you copied it from another field

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add title property to BarChartRodStackItem
2 participants