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

Replaced borderSide attribute with border attribute in BarChartRodData #1349

Closed

Conversation

MagdyYacoub1
Copy link
Contributor

Resolve issue #1194

Replaced borderSide attribute with border attribute in BarChartRodData

This enables specifying each border side separately.

@codecov
Copy link

codecov bot commented May 28, 2023

Codecov Report

Merging #1349 (2f6fb8e) into master (478e5e6) will increase coverage by 0.31%.
The diff coverage is 93.90%.

❗ Current head 2f6fb8e differs from pull request most recent head 8ec7e70. Consider uploading reports for the commit 8ec7e70 to get more accurate results

@@            Coverage Diff             @@
##           master    #1349      +/-   ##
==========================================
+ Coverage   86.09%   86.41%   +0.31%     
==========================================
  Files          45       45              
  Lines        2978     3054      +76     
==========================================
+ Hits         2564     2639      +75     
- Misses        414      415       +1     
Impacted Files Coverage Δ
lib/src/chart/bar_chart/bar_chart_painter.dart 93.36% <93.42%> (+1.18%) ⬆️
lib/src/utils/utils.dart 100.00% <100.00%> (ø)

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

@imaNNeo
Copy link
Owner

imaNNeo commented Jun 8, 2023

I think you need to write some unit-tests

@imaNNeo
Copy link
Owner

imaNNeo commented Jun 8, 2023

But your code is approved!

@imaNNeo
Copy link
Owner

imaNNeo commented Jun 14, 2023

It seems there are some conflicts, please rebase your branch again

@MagdyYacoub1
Copy link
Contributor Author

I'm sorry for not getting back to you sooner, but I was away for the past two weeks.

I resolved the conflicts. please, try again now.

@imaNNeo
Copy link
Owner

imaNNeo commented Jul 6, 2023

Hi, sorry for my late response. There are new conflicts.
It would be great if you fix them again.
Thanks!

…d of BorderSide

updated bar_chart_painter.dart
Added normalizeBorder function and defaultBorder attribute in utils.dart
Updated painter so the paint method be more constant with the paint method in the framework
Update documentation.
Remove format warning
Update documentation.
Remove format warning
…d of BorderSide

updated bar_chart_painter.dart
Added normalizeBorder function and defaultBorder attribute in utils.dart
Updated painter so the paint method be more constant with the paint method in the framework
Update documentation.
Remove format warning
Updated painter so the paint method be more constant with the paint method in the framework
Update documentation.
Remove format warning
@MagdyYacoub1
Copy link
Contributor Author

I resolved recent conflicts. Please provide me with some insights on why these conflicts happen so I can avoid that in the future.
Thanks in advance. @imaNNeo

CHANGELOG.md Show resolved Hide resolved
@imaNNeo
Copy link
Owner

imaNNeo commented Jul 31, 2023

There is an issue with BarSample 5
Please check it out, It will not happen in the master branch.
But it happens in your branch.
I am on mac with the latest version of flutter
(There might be an issue with the canvas in your changes)

bar.5.issue.mov

@imaNNeo
Copy link
Owner

imaNNeo commented Jul 31, 2023

I resolved recent conflicts. Please provide me with some insights on why these conflicts happen so I can avoid that in the future. Thanks in advance. @imaNNeo

Because we are constantly update the master branch (by pushing new commits). So you need to update your branch with the master.
The best way to do that is rebasing. Please read our contributing guidelines

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