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

Improvement/bartooltip item with text span children #554

Conversation

jrc2139
Copy link
Contributor

@jrc2139 jrc2139 commented Jan 22, 2021

redo of #544

This is a hold over for #72 in that you are able to better customize the text on the tooltip. I tried to change the code as little as possible by making the List field optional.

@jrc2139 jrc2139 force-pushed the improvement/bartooltip-item-with-text-span-children branch from 8d98eec to 3e3d11d Compare January 22, 2021 20:51
@imaNNeo imaNNeo changed the base branch from master to dev January 23, 2021 06:03
/// content of the tooltip, is a [text] String with a [textStyle].
BarTooltipItem(String text, TextStyle textStyle)
BarTooltipItem(String text, TextStyle textStyle, [List<InlineSpan> children])
Copy link
Owner

Choose a reason for hiding this comment

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

Why did you put List around [ ] ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i figured this is the easiest way to do it without breaking any compatibility. could have also done named. i have no preference

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so you can still call BarTooltipItem('something', TextStyle(fontSize: 12)) with no issue

Copy link
Owner

Choose a reason for hiding this comment

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

Okay please change it to named parameter.
Also, it will be great if you do the same in LineTooltipItem, and ScatterTooltipItem.

@imaNNeo
Copy link
Owner

imaNNeo commented Feb 17, 2021

Also please update one of our samples and use TextSpan (without adding any content like your gif).
Just add a new text using span instead of our previous method of adding texts.
For example, You can leave Saturday same as previous. and instead of the number below it. Add a customized TextSpan.
(I mean don't change any content)

@imaNNeo
Copy link
Owner

imaNNeo commented Feb 17, 2021

And the last step is updating the documents.
Update the bar_chart.md, scatter_chart.md, and the line_chart.md.

Thanks!

@imaNNeo imaNNeo force-pushed the dev branch 14 times, most recently from 15c9364 to 3d18dd0 Compare March 3, 2021 22:48
@imaNNeo
Copy link
Owner

imaNNeo commented Mar 16, 2021

And the last step is updating the documents.
Update the bar_chart.md, scatter_chart.md, and the line_chart.md.

Thanks!

Hi. I think you forgot to update the docs.
Also, you have a conflict.

@jrc2139
Copy link
Contributor Author

jrc2139 commented Mar 20, 2021

opening again for null-safety merge

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.

2 participants