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

Themeing / Global colors #52

Closed
petemill opened this issue Jul 26, 2018 · 4 comments
Closed

Themeing / Global colors #52

petemill opened this issue Jul 26, 2018 · 4 comments

Comments

@petemill
Copy link
Member

@petemill petemill commented Jul 26, 2018

There doesn't yet seem to be anywhere to define the global color palette.

I assume this would be useful to do as a default theme, so that multiple themes can be added or overidden by a consuming app.

This should be doable without having to pass round a theme={{ color: ... }} prop everywhere, via a ThemeProvider as described in the first example at: https://www.styled-components.com/docs/advanced#theming

// Define our button, but with the use of props.theme this time
const Button = styled.button`
  font-size: 1em;
  margin: 1em;
  padding: 0.25em 1em;
  border-radius: 3px;

  /* Color the border and text with theme.main */
  color: ${props => props.theme.main};
  border: 2px solid ${props => props.theme.main};
`;

// We're passing a default theme for Buttons that aren't wrapped in the ThemeProvider
Button.defaultProps = {
  theme: {
    main: 'palevioletred'
  }
}

// Define what props.theme will look like
const theme = {
  main: 'mediumseagreen'
};

render(
  <div>
    <Button>Normal</Button>

    <ThemeProvider theme={theme}>
      <Button>Themed</Button>
    </ThemeProvider>
  </div>
);

It should be ok to require that consumers of brave-ui always use a ThemeProvider and that brave-ui provides the default provider.

@petemill
Copy link
Member Author

@petemill petemill commented Aug 1, 2018

After a discussion with designers, here's the preferred approach to color variables / themeing:

  • Define a color palette: specific color choices that are on brand, and complimentary and we want to try to stick to., separately to the purpose of when to use the colors.
  • Then define our default theme colors: which use colors from the palette where appropriate.
  • We could have multiple themes - BraveLight, BraveDark, BATLight, Publishers, etc… (hopefully not all those, but this is an option for variation instead of completely different component styles).
  • Palette colors can be used directly, but most of the Components will use theme colors, e.g. see below

So Components (e.g Button) style (e.g. TextColor) would use Theme (e.g. BraveLight) colors for specific items (e.g. TextOnBrandColor) which may use palette colors (e.g. White300).

This allows:

  • A purposeful and consistent UX
  • Ease of maintenance and change
  • Ease of introducing theme variations (I’m pretty sure @BradRichter is keen on a dark mode for browser content areas)

Here’s an example of how this could be defined in the code, not sure how similar we are so far to this:

Palette
——
Red1
Red2
Red3
Purple1
Purple2
Purple3
Gray1 
Gray2
Gray3
White1
White2
White3

Theme - Brave
———
// general
BrandColor1 // brave
BrandColor2 // payments
BrandColor3 // when we feel crazy
PrimaryDark = Red1
PrimaryLight = Red3
PrimaryTextOnDark = White1
PrimaryTextOnLight = Gray2

// text specific
TextOnColor1
TextOnColor2
IconColor: Gray1
LabelColor: Gray2

// common specifics
DefaultBorder: Gray2
DropdownCarat: Gray2
ControlBorder: Gray3
ItemSelected: :#87ADD8;

// state colors (solid background, foreground text, borders, etc)
ErrorText: Red2 //error status text
ErrorLight: Red4 // error status item background
ErrorSolid Red1 // when used as a solid bg color (with e.g. white text)
Success…
Warning…


Theme - BraveDark
————
…

Theme - BATDefault (if we wanted a totally different look for Brave Payments vs. Brave)
———-
…

Note the descriptive description of colors use variants: PrimaryTextOnDark, ErrorSolid, ErrorLight, ErrorText
As well as the sample list of common specifics ^

@cezaraugusto
Copy link
Member

@cezaraugusto cezaraugusto commented Aug 10, 2018

ya agree I'm also not happy with the way we're using it. I'm considering allowing style as a prop for things such as margin, padding, etc and leaving theme for the real theming. Thoughts?

@petemill
Copy link
Member Author

@petemill petemill commented Aug 10, 2018

@cezaraugusto I don't think the Components should accept any explicit pixel values for padding, or anything. This defies the point of a component library and splits the style definition to multiple places. It precludes making single changes in the future because the consumption of the Components is tied to the current Implementation. Each Component should decide how it looks based on specific inputs (which as much as possible, and not tied to the exact look).

But if anything, they don't need a style prop - they can just have additional styles added by the consumer, as in:

const MyCardInstance = Card.extend`
   position: absolute;
   top: 10%;
   right: 0;
`
@cezaraugusto
Copy link
Member

@cezaraugusto cezaraugusto commented Aug 22, 2018

@petemill I guess we can close this?

@rossmoody rossmoody closed this Feb 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.