feat: custom chart colors support within existing colors prop

[adamhenson]

Description

This change is refactor of #836 to provide support for custom colors, but instead of the additive customChartColors prop - this ports functionality to the existing colors prop.

With this change, the colors prop can accept custom colors or tremor base colors ((Color | string)[]). customChartColors is removed in this change.

Example Usage

<LineChart
  {...props}
  colors={[
    // now supports any mix of tremor base colors...
    'emerald',
    'indigo',

    // ... and custom colors
    '#32a852',
    'custom-color-100',
  ]}
/>

Example safelist needed in Tailwind config to support the above implementation:

safelist: [
  // ..
  // ..
  // custom colors
  ...["[#32a852]", "custom-color-100"].flatMap((customColor) => [
    `bg-${customColor}`,
    `border-${customColor}`,
    `hover:bg-${customColor}`,
    `hover:border-${customColor}`,
    `hover:text-${customColor}`,
    `fill-${customColor}`,
    `ring-${customColor}`,
    `stroke-${customColor}`,
    `text-${customColor}`,
    `ui-selected:bg-${customColor}`,
    `ui-selected:border-${customColor}`,
    `ui-selected:text-${customColor}`,
  ]),
],

Updated Components

• AreaChart
• BarChart
• DonutChart
• LineChart
• ScatterChart
• SparkAreaChart
• SparkBarChart
• SparkLineChart

Related issues and PRs

• Related feat: custom chart colors support #836
• Related [Feature]: Support for custom hex colors #565
• Related Allow for custom colors #80
• Related feat: Add shades parameter to Donut Chart #640

What kind of change does this PR introduce?

Although this change updates the existing colors prop, the functionality is additive and doesn't break existing behavior.

• Bug fix (non-breaking change which fixes an issue)
• New Feature (non-breaking change which adds functionality)
• Refactor
• Build-related changes
• Other, please describe:

Does this PR introduce a breaking change? (check one)

• Yes
• No

How has This been tested?

Storybook stories have been added for all components demonstrating new support of custom colors in the colors prop.

The PR fulfills these requirements:

• It's submitted to the main branch
• When resolving a specific issue, it's referenced in the related issue section above
• My change requires a change to the documentation. (Managed by Tremor Team)
• I have added tests to cover my changes
• Check the "Allow edits from maintainers" option while creating your PR.
• Add refs #XXX or fixes #XXX to the related issue section if your PR refers to or fixes an issue.
• By contributing to Tremor, you confirm that you have read and agreed to Tremor's CONTRIBUTING.md guideline. You also agree that your contributions will be licensed under the Apache License 2.0 license.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
tremor-storybook	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 9, 2023 8:18pm

[severinlandolt]

This PR is what open-source maintainers dream of🫶 Will go to bed now and review it tomorrow!

[adamhenson]

Hah! Awesome, thanks @severinlandolt - appreciated 🙏

[github-actions]

🎉 This PR is included in version 3.12.0-beta-customcolors.2 🎉

The release is available on:

• npm package (@beta-customcolors dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[severinlandolt]

Hi @adamhenson, I just released a new beta (beta-customcolors) and testing it on our website. Two small questions:

1. Legend
   I think we do not need the customColor prop in the svg anymore, or am I missing something?
   

2. Interestingly, I use Tremor together with MDXRemote, and there is a TS error popping up. I am not 100% sure if this is a problem of Tremor or MDXRemote. I will do some tests.

Update: Adding export const colorValues: readonly string[] = [... and remove as const; assertion to the colorValues array does not work.
Error:

[adamhenson]

Thanks @severinlandolt! And thanks for testing. Yes, correct, we don't need customColor... I must have missed that one. Regarding the second issue - I wonder if there is a way I can reproduce 🤔

In the meantime, I'll open a branch against beta-customcolors to fix that first issue.

[adamhenson]

Would you be able to provide the full error @severinlandolt?

[severinlandolt]

Sure @adamhenson,

Type '{ Accordion: ForwardRefExoticComponent<tremor.AccordionProps & RefAttributes<HTMLDivElement>>; AccordionBody: ForwardRefExoticComponent<...>; ... 180 more ...; blockquote: (props: any) => JSX.Element; }' is not assignable to type 'MDXComponents | MergeComponents | null | undefined'.
  Type '{ Accordion: ForwardRefExoticComponent<tremor.AccordionProps & RefAttributes<HTMLDivElement>>; AccordionBody: ForwardRefExoticComponent<...>; ... 180 more ...; blockquote: (props: any) => JSX.Element; }' is not assignable to type 'MDXComponents'.
    Type '{ Accordion: ForwardRefExoticComponent<tremor.AccordionProps & RefAttributes<HTMLDivElement>>; AccordionBody: ForwardRefExoticComponent<...>; ... 180 more ...; blockquote: (props: any) => JSX.Element; }' is not assignable to type 'NestedMDXComponents'.
      Property 'colorValues' is incompatible with index signature.
        Type 'readonly ["slate", "gray", "zinc", "neutral", "stone", "red", "orange", "amber", "yellow", "lime", "green", "emerald", "teal", "cyan", "sky", "blue", "indigo", "violet", "purple", "fuchsia", "pink", "rose"]' is not assignable to type 'NestedMDXComponents | Component<any>'.
          Type 'readonly ["slate", "gray", "zinc", "neutral", "stone", "red", "orange", "amber", "yellow", "lime", "green", "emerald", "teal", "cyan", "sky", "blue", "indigo", "violet", "purple", "fuchsia", "pink", "rose"]' is not assignable to type 'NestedMDXComponents'.
            Index signature for type 'string' is missing in type 'readonly ["slate", "gray", "zinc", "neutral", "stone", "red", "orange", "amber", "yellow", "lime", "green", "emerald", "teal", "cyan", "sky", "blue", "indigo", "violet", "purple", "fuchsia", "pink", "rose"]'.ts(2322)
index.d.ts(30, 5): The expected type comes from property 'components' which is declared here on type 'IntrinsicAttributes & MDXRemoteSerializeResult<Record<string, unknown>, Record<string, unknown>> & { components?: MDXComponents | MergeComponents | null | undefined; lazy?: boolean | undefined; }'

Using npm i @tremor/react@3.12.0-beta-customcolors.4

I try to create a small repo to replicate the issue in the meantime

[adamhenson]

Also, with that said this could be a breaking change in types if Color type is being imported and expected to be colors: Color[]. I could see indexing being a problem. I am surprised the colorValues: readonly string[] change didn't work. But still digging in.

I'm wondering if we should do something like this and replace all occurrences of (Color | string)[].

- export type Color = (typeof colorValues)[number];
+ export type Color = (typeof colorValues)[number] | string;

EDIT: You can disregard the above - was just thinking. And I still don't think this should be a breaking change. The error is a little confusing because it says Property 'colorValues' is incompatible with index signature. while we don't use colorValues in other files (we can remove export). I'll look forward to the repro - if possible, otherwise continuing to brainstorm.

[severinlandolt]

Hey @adamhenson, I added you as an outside collaborator to this repo so feel free to directly push to https://github.com/tremorlabs/tremor/tree/beta-customcolors

Working on the reproduction now (ps: other repos of mine work fine)

[adamhenson]

Great, thanks @severinlandolt!

[adamhenson]

Another thought I had is maybe the build tool from the repro website doesn't like my type casting in this one.

export const getIsBaseColor = (color: Color | string) => colorValues.includes(color as Color);

But will wait to look at the repro.

EDIT: that doesn't really make sense either if it's built already. I'll just hold my horses 🏇

[severinlandolt]

Alright, sent you another invitation 📨 dev works, build not

Culprit: components/common/decoration/Markdown/Markdown.tsx

[adamhenson]

Perfect, thanks @severinlandolt 👀

[severinlandolt]

@adamhenson Feel free to make as many beta releases of https://github.com/tremorlabs/tremor/tree/beta-customcolors as you wish. Just prefix your commit with fix: and a new beta should be released

[adamhenson]

Oh awesome, thanks @severinlandolt! I might be able to get away with local testing via npm link but if not, that will be super helpful.

[adamhenson]

I have a theory that it's because I added export to export colorValues and it looks like it's being exported from the main package and the import is expected to be a component in this project. About to test this now.

From the built index.d.ts from the @tremor/react package.

I can remove that export statement anyways, since I'm no longer using it in other files. My theory is that this will fix it. More coming soon... just need to test.

This imports everything...

import * as tremor from "@tremor/react";

and then...

<MDXRemote
  {...source}
  components={{
    ...getRenderersByContext(context),
    ...tremor,
  }}
/>

[severinlandolt]

True, in the previous version the type in question was not exported!

[adamhenson]

@severinlandolt, yep - 3.12.0-beta-customcolors.5 fixes! I just removed the export - it was no longer needed anyways. I deleted my previous comment because that error might have been a bi-product of npm link. I ran the build with the new release and build completes successfully. I pushed to the bug repo too with the updated version.

[adamhenson]

Phew, thanks for all the help here @severinlandolt. And let me know if I can do anything else.

[severinlandolt]

No, I have to thank you! I will test the new beta in my repos asap🚀 Crazy what one single export can cause in side effects

[adamhenson]

Hah! Yep, the main entrypoint of the package exports everything from ./lib/inputTypes.

tremor/src/index.ts

Line 2 in 1744e4d

export * from "./lib/inputTypes";

And the repro project expects all those imports to be components 🤷 (or something like that... maybe all non-type exports)

// wildcards are a little dangerous for this reason
import * as tremor from "@tremor/react";

// ...

<MDXRemote
  {...source}
  components={{
    ...getRenderersByContext(context),

    // this is where i think the assumption is that all imports are components
    ...tremor,
  }}
/>

I get why this is like this... I mean you wouldn't want to have to update the imports explicitly anytime a component is added.