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

Add support for custom shapes #129

Conversation

mattprecious
Copy link
Contributor

@mattprecious mattprecious commented Mar 4, 2020

Closes #11

Main API change is a switch of the Shape type from an enum to an interface. With an interface, consumers are free to create whatever shape they want, so long as they can draw it using canvas operations. I also added rectangles and drawables as built-in shape implementations.

@mattprecious mattprecious force-pushed the mattp/2020-03-04/custom-shapes branch 2 times, most recently from c48cf99 to 8e38b73 Compare March 4, 2020 18:24
@DanielMartinus
Copy link
Owner

Hi @mattprecious, thanks for the PR! I've been holding back adding custom shapes to Konfetti in order to keep it performant. First of all I have to say that I'm really happy with the code that you delivered including the improvements. At first glance it looks great!

Give me some more time to review the code and play with some custom shapes. Main goal is to make sure everything stays performant so I also want to run some tests but looking at the implementation I think it'll be fine! Really cool

@mattprecious
Copy link
Contributor Author

Yep, keeping the performance is definitely important. I tried to keep it as lightweight as possible by sticking with the canvas APIs, but let me know if you find anything that needs addressing.

It's definitely possible to build a poorly-performing shape implementation, but I think that's fine as long as the built-in ones work well.

@mattprecious
Copy link
Contributor Author

One thing I noticed with this change is that since you can now use shapes that aren't symmetrical both vertically and horizontally, it's very clear that they only flip along the horizontal axis.

@DanielMartinus
Copy link
Owner

DanielMartinus commented Mar 6, 2020

One thing I noticed with this change is that since you can now use shapes that aren't symmetrical both vertically and horizontally, it's very clear that they only flip along the horizontal axis.

Just checked that indeed and you're right. I think that's fine for now. Letting people know the rotation effect looks best with symmetrical shapes is good enough for now. Adding this is already a major step forwards and requested by a lot of people. 🙂

@mattprecious mattprecious force-pushed the mattp/2020-03-04/custom-shapes branch from 8e38b73 to 4fb5a21 Compare March 6, 2020 20:19
@DanielMartinus
Copy link
Owner

DanielMartinus commented Mar 7, 2020

@mattprecious Can I or do you want to rebase the branch on master?
Also, anything else you want to check or discuss before moving forward merging this branch?

@mattprecious
Copy link
Contributor Author

Go for it!

@DanielMartinus DanielMartinus merged commit 2622cdc into DanielMartinus:master Mar 7, 2020
@mattprecious mattprecious deleted the mattp/2020-03-04/custom-shapes branch March 7, 2020 17:55
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.

New shapes
2 participants