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 convert to outlines #431

Open
wants to merge 5 commits into
base: develop
from

Conversation

@WCByrne
Copy link
Contributor

commented Mar 25, 2019

Addresses #408


### Returns

An array of new [Layer](#layer) objects representing the outlines of this layer.

This comment has been minimized.

Copy link
@WCByrne

WCByrne Apr 9, 2019

Author Contributor

Might be nice to clarify when this has more than one layer. I'm still not sure when that would be.

@mathieudutour

This comment has been minimized.

Copy link
Contributor

commented Apr 9, 2019

Ah sorry, I didn't comment on this.

It feels a bit weird to me to have this method on a Layer instance. Ideally, I'd prefer to have something like Layer.fromOutlines(layer) but I'm not sure where it would go.

But you are right that it can sometimes create multiple layers (when a shape has multiple borders IIRC). So I'm still not sure

@WCByrne

This comment has been minimized.

Copy link
Contributor Author

commented Apr 9, 2019

My opinion: The current name and location (layer.layersByConvertingtoOutlines) is more clear than having it on Layer. Layer.fromOutlines(layer) actually feels a little more awkward to me. If anything, maybe something like outlineLayersFromLayer(layer) but that still reads weird.

@mathieudutour

This comment has been minimized.

Copy link
Contributor

commented Apr 9, 2019

yeah, I meant more about having it as a class method (eg. more like a constructor) rather than an instance method

@WCByrne

This comment has been minimized.

Copy link
Contributor Author

commented Apr 22, 2019

Any more thoughts on naming for this?

@WCByrne

This comment has been minimized.

Copy link
Contributor Author

commented Apr 30, 2019

@mathieudutour Let me know if and how you want me to update this. Happy to do whatever you think makes the most sense.

@mathieudutour

This comment has been minimized.

Copy link
Contributor

commented Apr 30, 2019

Yeah sorry, I need to find some time to play with a few different namings. Sorry for the delay :/

@WCByrne

This comment has been minimized.

Copy link
Contributor Author

commented Apr 30, 2019

No worries, just remembered this had been sitting here for a while. This is an easy one to drop in to the internals for now.

@iliakan

This comment has been minimized.

Copy link

commented Aug 10, 2019

Is there a way to convert to outlines for JS Sketch plugin right now?

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.