ment-mx / Prism

Creates a beautiful artboard color palette with all your 'Document Colors' and their respective color label in a variety of formats.

MIT License

1.01k stars 47 forks source link

add support for document color #45

Open griffin-stewie opened 4 years ago

griffin-stewie commented 4 years ago

I added support for named document colors.

Steps to get name:

Try to get name from document color
Try to get alias color name
Finally, use the color classifier.

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 to MSColorAsset base.

It could be significant change so I didn't change now, but we may consider to change design.

Related Issue

43

custa1200 commented 4 years ago

Any chance these pull request could be actioned?

custa1200 commented 4 years ago

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 commented 4 years ago

As an example the image has a transparent color which is White with 0 alpha, it is also naming the White at the bottom with 100 alpha Transparent. There is 2 greys as you can see too, the second one is named "Selected" in the named color list but it gets assigned Grey a second time.

griffin-stewie commented 4 years ago

@custa1200 Is what you're calling "Named Color" the content of this link? or what you named them on the Prism's artboard?

griffin-stewie commented 4 years ago

@custa1200 I'm starting to get it.

As an example the image has a transparent color which is White with 0 alpha, it is also naming the White at the bottom with 100 alpha Transparent.

This case of bug is able to be fixed it easily.

There is 2 greys as you can see too, the second one is named "Selected" in the named color list but it gets assigned Grey a second time.

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.

custa1200 commented 4 years ago

It's "almost" perfect. I also made a formatter for the Hex + Alpha too, I might do a pull request for that.

custa1200 commented 4 years ago

@griffin-stewie would the second one be worth doing and looking to add it to a 2.0 release?

griffin-stewie commented 4 years ago

@custa1200 Unfortunately, I'm just a contributor so I can't judge the release. I think I need to take a closer look at the sphere of influence where I feel it's a problem. I am willing to make changes to this Pull Request if the problem is less severe than I was concerned about.

custa1200 commented 4 years ago

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?

griffin-stewie commented 4 years ago

@custa1200 I pushed some commits to fix problem you mentioned. Could you try this?

custa1200 commented 4 years ago

I tried it out and it worked a treat @griffin-stewie I would say that handles all the color naming issues for me.

griffin-stewie commented 4 years ago

@custa1200 I’m glad to hear it! Thank you for testing it.