segmentio / evergreen

🌲 Evergreen React UI Framework by Segment
https://evergreen.segment.com
MIT License
12.39k stars 832 forks source link

Suggestions for Migration/Theming chapters in the docs #1239

Closed jasonbarry closed 3 years ago

jasonbarry commented 3 years ago

Hi! Thanks so much for providing this library. I just migrated our codebase from v5 → v6 and thought it would be useful to share some gotchas I ran into around theming:

It took me a long time to realize why modifying the fontWeight of my Button's baseStyle wasn't working: I had set height={40} in numerous instances of my app, and this property was overwriting my custom font weight from being applied. Going through my app and replacing height={40} with size="large" allowed my custom font weight to appear. There's only one line in the migration chapter that mentions this:

Additionally, we've unified the sizing API so SegmentedControl also accepts a size prop just like Button, TextInput, Group, etc.

I think that more attention should be brought to the size prop.

It's not clear to me when a rule should be placed in baseStyle vs when it should be placed in appearances.default. For example, this lets me set a background color on Select:

Select: {
  baseStyle: {
    background: 'linear-gradient(to bottom, #FFFFFF, #F4F5F7)',
    fontWeight: 500,
  },
  appearances: {
    default: {
      color: '#425a70',
    },
  },
},

But this does not:

Select: {
  baseStyle: {
    fontWeight: 500,
  },
  appearances: {
    default: {
      background: 'linear-gradient(to bottom, #FFFFFF, #F4F5F7)',
      color: '#425a70',
    },
  },
},

The Theming API Overview section is ambiguous in detail: "Note that some components don't have this." Which ones?

It was helpful for me to log classicTheme to console to know where to extend rules. It would be helpful if more details about the shape were present in the docs rather than repeating /* ... */.

It's much easier to extend a theme with deepmerge rather than spreading objects over and over:

const myTheme = {
  components: {
    Button: {
      appearances: {
        superdanger: {
          background: 'indianred',
        },
      },
    },
  },
}

const extendedTheme = merge(classicTheme, myTheme)
// console.log(extendedTheme)

export default function MyThemeProvider(props: Props): JSX.Element {
  return <ThemeProvider value={extendedTheme}>{props.children}</ThemeProvider>
}

That's all for now, thanks again for a relatively painless upgrade!

akleiner2 commented 3 years ago

@jasonbarry This is all super helpful -- deepmerge especially is helpful. Might be worth us exporting extendTheme() that effectively does that under the hood.

If you have more examples / ideas for theming docs, please don't hesitate to submit a PR - we're open for ideas! Theming is a new concept for us in EG. Going to close this for now - definitely top of mind.