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

feat: support custom accent colors, fix preset contrast issues #267

Merged
merged 6 commits into from
Apr 27, 2022

Conversation

markdalgleish
Copy link
Contributor

@markdalgleish markdalgleish commented Apr 26, 2022

The accentColor option on the built-in themes now accepts arbitrary strings rather than the name of a built-in preset.

To allow consumers to adjust the text contrast, there is now also an accentColorForeground option.

For example:

import { RainbowKitProvider, darkTheme } from '@rainbow-me/rainbowkit';

const App = () => {
  return (
    <RainbowKitProvider
      theme={darkTheme({
        accentColor: '#FFFF00',
        accentColorForeground: '#000',
      })}
    >
      {/* Your App */}
    </RainbowKitProvider>
  );
};

The built-in accent color presets are now available via an accentColors property on each theme function. For example:

import { RainbowKitProvider, darkTheme } from '@rainbow-me/rainbowkit';

const App = () => {
  return (
    <RainbowKitProvider
      theme={darkTheme({
        ...darkTheme.accentColors.pink
      })}
    >
      {/* Your App */}
    </RainbowKitProvider>
  );
};

This PR also increases the foreground text contrast for some of the accent color presets used for the dark themes. Since our color palette is already quite soft, the accent colors below were picked by eyeballing them and subjectively deciding which ones were a little too low contrast against white text, but I'm happy to change this if there's disagreement. The main thing is that, either way, we now have a mechanism for easily changing this per accent color preset.

Before:

Screen Shot 2022-04-26 at 10 34 36 am

Screen Shot 2022-04-26 at 10 34 24 am

Screen Shot 2022-04-26 at 10 34 11 am

After:

Screen Shot 2022-04-26 at 10 39 45 am

Screen Shot 2022-04-26 at 10 39 35 am

Screen Shot 2022-04-26 at 10 39 20 am

This PR also removes the yellow preset since it can't be made consistently accessible across all themes, notably in the light theme:

Screen Shot 2022-04-26 at 10 34 49 am


Closes RNBW-3344 and RNBW-3341.

@markdalgleish markdalgleish requested a review from a team as a code owner April 26, 2022 00:47
@linear
Copy link

linear bot commented Apr 26, 2022

RNBW-3344 Support custom accent colors in built-in themes

This should be done after RNBW-3341.

API concept:

darkTheme({
  accentColor: {
    value: 'yellow',
    foregroundText: 'black'
  }
})

(property names need some thought, but this is the idea of how it works mechanically)

RNBW-3341 Fix contrast of accent colors

@vercel
Copy link

vercel bot commented Apr 26, 2022

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

Name Status Preview Updated
rainbowkit-example ✅ Ready (Inspect) Visit Preview Apr 27, 2022 at 10:24PM (UTC)
rainbowkit-site ✅ Ready (Inspect) Visit Preview Apr 27, 2022 at 10:24PM (UTC)

@peduarte
Copy link
Contributor

lovely jubbly 👌

in the example, we have defined the custom accent color as:

accentColor: 'yellow',
accentColorForeground: 'black',

which renders like this:

image

one thing that confused me a bit, is that black is actually used as a "background" color of the main cta but it's called accentColorForeground

in order to remove this confusion, what do we think about calling it accentColorSecondary instead?

@markdalgleish
Copy link
Contributor Author

markdalgleish commented Apr 27, 2022

@peduarte I'm using an inverted example in light mode, so that screenshot is actually using { accentColor: 'black', accentColorForeground: 'yellow' }. The foreground color is only ever used for the text. I might update the example custom color to something that doesn't cause this confusion.

@peduarte
Copy link
Contributor

@markdalgleish ah ok, cool!

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

Successfully merging this pull request may close these issues.

None yet

3 participants