sketch-dark-mode icon indicating copy to clipboard operation
sketch-dark-mode copied to clipboard

Add ability to have multiple color variables with same HEX code

Open p-mercury opened this issue 2 years ago • 2 comments

This PR adds the ability to have multiple color variables with the same HEX code and still be able to assign each one a separate dark mode value.

Closes #33

And fixed problem causing dark mode colours not to be applied to a symbols tint.

p-mercury avatar Mar 28 '22 18:03 p-mercury

Hey 👋🏻 Thank you for your contribution!

I would like to merge your PR, however there is a major issue with your implementation, it can only get the colors from the current local document, the plugin also supports Libraries so it's important to be able to get colors from there as well.

I would suggest to keep using the baseColors array and modify the function getDocumentColors in the utils.js file. Both local documents and Libraries get the used colors from that function, that means that if you modify that function the plugin will still support Libraries.

I'm not sure if doc.sketchObject.documentData().sharedSwatches() is equivalent to doc.swatches, if both return the same data then you don't even need to modify that function, in that case you only need to get the swatch object as follows:

const swatch = baseColors.swatchWithID(color.swatchID())

If they are not equivalent then you can do something like this within the getDocumentColors function:

export const getDocumentColors = (doc) => {
  const legacyColors = doc.colors

  // hasSketchObjectSupport should be a function that detects wheter Sketch has support for `sketchObject` or not
  if (hasSketchObjectSupport()) {
    return doc.sketchObject.documentData().sharedSwatches()
  }

  if (hasSketchColorVariablesSupport() && doc.swatches.length > 0) {
    // Renames Color Variables based on legacy colors
    if (legacyColors.length > 0) {
      doc.swatches.forEach((swatch) => {
        const swatchColorName = swatch.name.substr(3)
        const foundLegacyColor = legacyColors.find((legacyColor) => {
          return swatchColorName === legacyColor.name
        })

        if (foundLegacyColor) {
          swatch.name = foundLegacyColor.name
        }
      })
    }

    return doc.swatches
  }

  return legacyColors
}

I would suggest also to create a function called hasSketchObjectSupport to detect wheter Sketch has support for sketchObject or not, you can assign the returned value of that function to a variable at the top of the file and use it like this:

const sketchObjectSupport = hasSketchObjectSupport()
// ...
style.color = sketchObjectSupport ? switchColor(style.sketchObject.color()) : switchColor(style.color)

What do you think?

eddiesigner avatar Apr 03 '22 15:04 eddiesigner

I might have gone a bit overboard with this, but here we are 🫣

So to solve the library support issue I redesigned the plugin in such a way that you can now have a dark theme for your document colours and all libraries at once instead of having to choose one of them at a time.

This latest version uses a different key for the settings so it won't override you old variables when you trying this one out.

This is just a draft and I still have to do a ton of cleaning up, and tomorrow I'll write a longer comment explaining all of my changes. But it is already functional so if you want to try it out go right ahead.

The only disadvantage for this completely new way of doing things is that it won't be able to support documents and libraries that don't use the new color variables. For me personally this is not an issue, I usually keep everything up to date, but i'm not sure how keen you are on having backwards compatibility.

Here is an example of a canvas that uses both document colours and library colours at once:

Screenshot 2022-04-04 at 21 42 42 Screenshot 2022-04-04 at 21 40 46

p-mercury avatar Apr 04 '22 19:04 p-mercury