diegoveloper / flutter_percent_indicator

Flutter percent indicator library
BSD 2-Clause "Simplified" License
691 stars 206 forks source link

Radius parameter is actually a diameter #33

Closed shorben07 closed 2 years ago

shorben07 commented 5 years ago

CircularPercentIndicator has a property final double radius; but in implementation, it is used as a diameter:

Container(
        height: widget.radius + widget.lineWidth,
        width: widget.radius,

Here we get the real radius by dividing by two

CirclePainter(
              radius: (widget.radius / 2) - widget.lineWidth / 2,

Please read more info at link https://www.mathlearnit.com/circle-circumference.html circle-circumference-radius-diameter

diegoveloper commented 5 years ago

You are right, I think we need to update the name of the field and the doc, the issue is it will be a breaking change

shorben07 commented 5 years ago

@diegoveloper It is not really a big issue. For me, it took a couple of minutes to figure out why the circle is smaller than it should be. Thanks for the great library!

xuanswe commented 4 years ago

the issue is it will be a breaking change

You could introduce new field diameter and deprecated the field radius, then in DartDoc asking people to migrate to diameter and warn them radius will be removed in the next major release.

diegoveloper commented 4 years ago

PRs are open :)

On Thu, Jul 2, 2020, 1:50 PM Nguyen notifications@github.com wrote:

the issue is it will be a breaking change

You could introduce new field diameter and deprecated the field radius, then in DartDoc asking people to migrate to diameter and warn them radius will be removed in the next major release.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/diegoveloper/flutter_percent_indicator/issues/33#issuecomment-653168161, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABFL3UHTVMEQ2WJT33WZ3RLRZTJHZANCNFSM4IGA45PQ .

slimlime commented 3 years ago

the issue is it will be a breaking change

You could introduce new field diameter and deprecated the field radius, then in DartDoc asking people to migrate to diameter and warn them radius will be removed in the next major release.

^

I ended up hard breaking change updating a fork (gotta go fast!) along with a slew of other refactors

Might make time far in the future for a more maintainable release schedule (and also backport re-up nullsafety)

Radius vs diameter dissonance was a fun discovery when mock testing UI with boxes

diegoveloper commented 3 years ago

@slimlime if you want to send a PR, you are welcome.

nvshah commented 2 years ago

@diegoveloper Attempted a simple attempt for aforementioned issue (with minimal changes) accounting diameter instead of radius whilst building the Widget

PR ref :- https://github.com/diegoveloper/flutter_percent_indicator/pull/140