-
Notifications
You must be signed in to change notification settings - Fork 47
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 document color #45
base: master
Are you sure you want to change the base?
add support for document color #45
Conversation
Any chance these pull request could be actioned? |
One issue with this @griffin-stewie is that if I have 2 named colors that happen to have the same hex value, the second color will be named the same as the first one. when using named colors there is always the chance of having multiple versions of the same color with the same name. |
@custa1200 |
@custa1200
This case of bug is able to be fixed it easily.
In this case, it will not be easy to fix due to the impact of the original implementation. I would try to fix but it would be breaking change. |
It's "almost" perfect. I also made a formatter for the Hex + Alpha too, I might do a pull request for that. |
@griffin-stewie would the second one be worth doing and looking to add it to a 2.0 release? |
@custa1200 Unfortunately, I'm just a contributor so I can't judge the release. |
The owner hasn’t picked up your pull requests for a while :( should I spin this out to an issue to track outside this PR? |
@custa1200 |
I tried it out and it worked a treat @griffin-stewie I would say that handles all the color naming issues for me. |
@custa1200 I’m glad to hear it! Thank you for testing it. |
I added support for named document colors.
Steps to get name:
I added to edit a name of document color inside
colorNameChanged
as well.Consideration
My understanding is there was no "named document color" feature on Sketch itself when Prism came out. However now Sketch has "named document colors" since Sketch v53. So how to manage colors inside Prism is time to change
MSColor
based toMSColorAsset
base.It could be significant change so I didn't change now, but we may consider to change design.
Related Issue
#43