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

Doesn’t work with library symbol overrides #8

Closed
timohofmeijer opened this issue Apr 10, 2019 · 7 comments
Closed

Doesn’t work with library symbol overrides #8

timohofmeijer opened this issue Apr 10, 2019 · 7 comments

Comments

@timohofmeijer
Copy link

To reproduce, add any library symbol with a text override to a new document and try to apply faker data to it.

Tested this in Sketch 54.

@paintedbicycle
Copy link
Owner

Thanks for reporting this. I haven't tested with layers from external libraries, so you may be right that it's a bug in Sketch Data Faker. I'll take a look.

Can you tell me one thing though - have you used regular "Sketch Data" on a layer imported from a Library? And does that work? So, if you do exactly the same thing you tried with Sketch Data Faker, but instead you choose "Data -> Sketch Data -> Names" - will that work on the same layer that Sketch Data Faker is not?

Thanks!

@timohofmeijer
Copy link
Author

timohofmeijer commented Apr 11, 2019

Yes, I tested this.

  • Other ‘Sketch Data’ does work as expected when overriding library symbol instances.
  • Manual (not name based) ‘Faker Data’ overrides work neither.

@paintedbicycle
Copy link
Owner

Ok great. Hopefully they expose the ability to set this in the API. I will take a look this weekend. If it's possible to fix, it should be done in a few days.

@paintedbicycle
Copy link
Owner

paintedbicycle commented Apr 16, 2019

Hi @timohofmeijer - a quick update.

I did some debugging this weekend and recreated your issue. Two things I learned:

  1. Sketch Data Faker is not finding the layer correctly, when the layer is a from a library. I do a check here https://github.com/paintedbicycle/sketch-data-faker/blob/master/src/faker.js#L25 that currently retrieves the layer by looking up the id of either a normal layer, or, in the case of a symbol, it uses the symbol override instead. It looks like I need to add yet another check here for library instances, passing the correct id. I thought it would already be taken care of, but apparently not. I'll have to research a little more to get this working.

  2. One potential issue I see going forward is with Automatic Mode which requires changing the layer name. I'm not certain you can rename layers inside libraries that you've imported. Do you know if that's true (in general with Sketch) or are layer names locked and you need to detach from the library before you can rename them?

So I can likely get Manual Mode (the one with quick overrides) working, but it might be harder to get the full automatic mode working unless the Sketch user has the ability to edit the names of the original library layer. I'll definitely want to get this working with libraries, so it might mean a much larger plugin rewrite. Anyway, more research needs to be done...perhaps I can indeed change the layer name programatically. Just not sure yet.

I'll keep going on this next weekend.

@timohofmeijer
Copy link
Author

Hi Paul, grateful for your efforts! One could of course rename layers in a self managed library. With an external library (e.g. iOS) this would require a fork and is likely not desirable. I personally would prefer applying data manually even though I manage the library myself. I could fork and have a look at the code this weekend, if you could let me know if you are not already on this, so we don't spend duplicate time : )

@paintedbicycle
Copy link
Owner

@timohofmeijer Just pushed a version live for testing. You should now be able to override the text in a library layer using Manual mode.

Automatic mode works too, but it sets the layer name on the parent. This is the only editable layer name and actually happens automatically. However, since the layer that gets the new content is only the layer that is selected, it updates the correct layer with the text.

I'll have to investigate more, but let me know what you think

@paintedbicycle
Copy link
Owner

Hi @timohofmeijer I broke Library symbol overrides in a previous release, sorry about that. It is now fixed with 0.7.0. If you're still using it, I'd love to hear if it worked for you.

Thanks!

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

No branches or pull requests

2 participants