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

Integrate some widgets with painter #79

Closed
wants to merge 12 commits into from
Closed

Integrate some widgets with painter #79

wants to merge 12 commits into from

Conversation

payam-zahedi
Copy link
Contributor

Hi I just widgets some files and integrated them with CustomPainter. List of widgets that have been Changed:

  • SpinKitSquareCircle
  • SpinKitPumpingHeart
  • SpinKitPulse
  • SpinKitDoubleBounce

also, I Removed itemBuilder of these widgets.
If any changes needed please let me know. Thank you

@jogboms
Copy link
Owner

jogboms commented May 27, 2020

Hey @payam-zahedi

Nice work. I appreciate the fact you did this anyway, real good. Pending from our conversation on #55, it became obvious we would have to remove the itemBuilder feature which I remember some members of the community personally asking for.

I played with a quick approach last weekend using a RenderObject and painting the child (itemBuilder) when it exists otherwise, drawing the animation. It seemed to work as one would expect. I only tried it on one case.

Another might be to make a v5 release without the itemBuilder but keep the v4 maintained as well.

What do you think as well. cc @ened @CaiJingLong @aagarwal1012

@jogboms jogboms self-requested a review May 27, 2020 05:42
@ened
Copy link
Contributor

ened commented May 28, 2020

In our project we are not using itemBuilder, however - others might.
I understand how itemBuilder would not be usable in a painter, but surely we could have a itemPainter parameter with a signature like this:

typealias ItemPainter = Function(Canvas canvas, Size size);

.. and pass that as a function to the SpinKit APIs?

The Function could even take an optional parameter for the time or animation progress, so that the painting & animation are dynamic?

WDYT @payam-zahedi ?

@jogboms
Copy link
Owner

jogboms commented May 29, 2020

Thanks @ened, that indeed solves the issue of modifying the kit from the outside. I love the approach I do hope the users already comfortable with composing widgets would not see the canvas as an added level of complexity. I would indeed love to do this way. 😅

@jogboms jogboms linked an issue May 29, 2020 that may be closed by this pull request
@jogboms jogboms added this to the v5.0.0 milestone May 29, 2020
@payam-zahedi
Copy link
Contributor Author

payam-zahedi commented May 29, 2020

@ened thanks for this great approach, but I think we have limitation for it. this approach is very nice when we will paint a kit with a single widget(painter) like spinning-circle. but what happened if we want to paint kit with multiple widgets(painters) like Wake.
should we give multiple itemPainter from the user? something like this:
typealias ItemPainter = Function(Canvas canvas, Size size, int index);
or the user should paint all the widgets in a single itemPainter. I think this gonna be very hard for the user when he wants to paint widget from outside.
what do you think @jogboms ?

@jogboms
Copy link
Owner

jogboms commented May 30, 2020

Yes @payam-zahedi it did cross my mind in order to maintain "parity" in some way the index would be necessary indeed.

@payam-zahedi
Copy link
Contributor Author

ok @jogboms, should I change my PR?
or you would work on it to integrate it with the new item builder approach?

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.

Custom Painters!
3 participants