Add convert to outlines

[WCByrne]

Addresses #408

[WCByrne]

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

[mathieudutour]

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]

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]

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

[WCByrne]

Any more thoughts on naming for this?

[WCByrne]

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

[mathieudutour]

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

[WCByrne]

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]

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

Thank you.

[WCByrne]

This PR needs to address this issue that was brought up in Slack 👇 . @christianklotz any insight on how this can be achieved in code? I'd be happy to implement and get this PR moving again.

It looks like converting text to outlines is filling in letters in unexpected ways. I believe this started with 58 according my user’s reports.

Response: altering the fill type from Non-Zero to Even-Odd will likely achieve the intended result there (via the cog button in the Fills section within the Inspector)

[christianklotz]

This is going to be fixed in Sketch 60. Once out it'd be a good time to pick up this PR again.

[christianklotz]

The issue here is that this would actually fail on any layer that doesn't support conversion to outlines, e.g. symbol instances, groups, etc. There's a method canConvertToOutlines which returns true for anything that can be converted, posing the question of how this would be translated here. Does it only return itself wrapped in an array or null/undefined?

[christianklotz]

Suggested change

	layersByConvertingToOutlines() {
	toOutlines() {

Maybe toOutlines, similar to String.toUppercase would suffice?

[WCByrne]

Makes sense to me.

[WCByrne]

As for failing when canConvertToOutlines is false, do the internals return nil or itself? Should that check function be adde here as well?

[christianklotz]

@WCByrne it gets never called because it's only implemented for layer subclasses that support it.

[WCByrne]

got it. I feel like returning nil then, or maybe even throw an error to make it clear why it didn't work. If you are trying to convert something to outlines, getting that layer back in an array seems like it could be confusing

[christianklotz]

Returning nil works for me. Throwing an error is a bit harsh, especially if the method is on Layer. An alternative could be to put it on Text and ShapePath only?

[WCByrne]

If those are the only 2 that support it, that works for me. I was just kinda going with what the internals seemed to do. FWIW, I only use it for text layers so this is a bit of a none issue for my needs.

[christianklotz]

👍 … feel free to make the change so only the two classes have the method and I'll review.