Skip to content
This repository has been archived by the owner on Dec 13, 2018. It is now read-only.

[Proposal] Advanced style overrides/theming for reusable components #202

Closed
schnerd opened this issue Jun 30, 2017 · 19 comments
Closed

[Proposal] Advanced style overrides/theming for reusable components #202

schnerd opened this issue Jun 30, 2017 · 19 comments

Comments

@schnerd
Copy link

schnerd commented Jun 30, 2017

Problem

Glamorous offers theming, and while this is certainly useful I think there's still a missing solution for style overrides in complex reusable components.

As an example, let's consider a <Datepicker> component (like react-datepicker). We'd like to expose a theme prop where users can customize the theme colors. This is easy, we can pass the user theme to <ThemeProvider> and then use these values to style the internals of our reusable component like this:

const Day = glamorous.div((props, theme) => ({
  borderColor: theme.mainColor
}))

This works well enough for the simple case, but what if someone wants to change a style we haven’t initially anticipated? For example, adding a border radius, changing font-face/weight/size, or removing white space across all elements for a more condensed view. Our component could quickly grow into the following:

const Day = glamorous.div((props, theme) => ({
  backgroundColor: theme.themeColorDark,
  padding: theme.dayPadding || '4px',
  borderRadius: theme.dayRadius || '2px',
  font: props => theme.dayFont,
  // etc...
}))

You might even consider spreading in a style overrides object, but you‘d also need to spread into any pseudo-classes since spreads don’t merge objects recursively:

const Day = glamorous.div((props, theme) => ({
  padding: '4px',
  ':hover': {
    backgroundColor: theme.themeColorDark
    ...theme.dayHover
  },
  ':first-child': {
    padding: '2px',
    ...theme.dayFirstChild
  },
  ...theme.day
}));

Maintaining solutions like this can be quite tedious as a developer, and we can likely offload some of this to the library.

Proposed Solution

Let's extend glamorous's theming layer by allowing optional theme identifiers on glamorous components, and internally glamorous will do the job of merging overrides with the base component styles.

Revisiting our date picker example, we can use this new strategy:

const Day = glamorous.div('DatePickerDay', {
  borderColor: '#ccc'
})

Here we’ve added an argument that is an optional theme identifier for the element (alternatively this could be a themeName prop passed to <Day>). Users of my datepicker component can target this identifier and others with a style override object:

const datePickerStyles = {
  DatePickerDay: (props, theme) => ({
    borderColor: props => props.selected ? '#00bbff' : '#bbb'
  }),
  DatePickerHeader: {
    padding: '4px',
  },
  // etc.
};

<DateRangePicker styles={datePickerStyles} />

Glamorous would do the heavy lifting of merging the overrides into the Day components styles, using a recursive merge so that pseudo-classes work properly. Note that we maintain the ability to use dynamic styles based on props, although we may want a mechanism to select which props become a part of the public API.

Giving all components theme identifiers also introduces other conveniences, for example we could even reference them inside a descendant combinator from a parent component:

// Parent.js
const Parent = glamorous.div('Parent', {
  ':hover $Child': {
    opacity: 1;
  }
});

// Child.js
const Child = glamorous.div('div', 'Child', {
  opacity: 0;
});
(This is an alternative API to current WIP in #136.)

When a Child renders, any rules found in a parent component that contain $Child will also be injected (the $ prefix denotes a theme identifier, syntax inspired by jss-nested). This is obviously separate functionality from the main API being proposed here and not required, just figured I'd mention it since #136 is being worked on.

Why?

This is one of the last pieces of the puzzle for me in CSS-in-JS. We are big users of various CSS-in-JS libraries at Uber, and this is easily the biggest pain point I've seen developers run into when working with our reusable component libraries. It also seems like an unsolved problem in the wider community [1, 2, 3, 4]. Finding a solution would be a major step forward for using css-in-js in reusable components libraries, as developers will have complete flexibility over re-styling for their use case.

@schnerd
Copy link
Author

schnerd commented Jun 30, 2017

cc @taion given your previous post. curious to hear your thoughts on an API like this.

@kentcdodds
Copy link
Contributor

Hi! Thanks for the issue. I haven't had time to give it a thorough look yet, but at first glance I think this may solve your issue: https://rc.glamorous.rocks/examples#Accept-style-overrides

@schnerd
Copy link
Author

schnerd commented Jun 30, 2017

Oh nice! I searched far and wide but didn't see that example anywhere (guessing rc.glamorous.rocks isn't fully released yet?).

Anyway, that is definitely a viable solution. It is more verbose than the proposed API, but it ultimately provides the same functionality. IMO it'd be great to see this functionality/helper built into glamorous core or released as a glamorous plugin rather than relying on users to define their own helper, but I suppose the helper is only a few lines of code and if featured prominently in the glamorous docs (looks like it will be), then it should still be straightforward for users.

Up to you–feel free to close this if you don't think there's much to be gained here beyond the example you shared.

@taion
Copy link

taion commented Jun 30, 2017

My understand of glamorous is quite limited, so take this with a grain of salt, but:

The solution in the OP looks more like what I was picturing than the solution at https://rc.glamorous.rocks/examples#Accept-style-overrides.

The problem I was thinking of is that something like a <Button> might be used in multiple contexts that need to provide contextual styles. In the "style override" example, <Tagline> is closely coupled to <UserCard>, and seems like it's not "generic" in that sense, though that might just be a limitation of that example – though that said, using a theme provider for everything you might want to override feels like it'd be clunky.

@kentcdodds
Copy link
Contributor

Yeah, I think that the reusable/generic argument is solved by glamorous's normal use cases with the css prop, and composible classNames. You would only really ever do the style overrides thing for a situation where you want to expose overriding capabilities to a group of components (like an Autocomplete, or Tabs or something). Does that make sense?

So normally you'd do something like:

const Button = glamorous.button({/* basic styles */}, (props) => ({/* dynamic styles you think of ahead of time that are generic */})

<Button css={{ /* styles that are specific to your use-case */ }} />

These all merge together in a really nice and predictable way :)

I hope that helps.

@kentcdodds
Copy link
Contributor

Oh, and with regards to baking the accept-style-overrides idea into glamorous, I don't think that's something I'm willing to do, but I do see value in having a small library expose something like that. Seems like a pretty useful pattern.

@schnerd
Copy link
Author

schnerd commented Jul 2, 2017

Yeah, I think that the reusable/generic argument is solved by glamorous's normal use cases with the css prop, and composible classNames. You would only really ever do the style overrides thing for a situation where you want to expose overriding capabilities to a group of components (like an Autocomplete, or Tabs or something). Does that make sense?

Yup, this proposal was more focused on components like autocomplete, modals, tabs, pagination, datepickers etc. (let's call these "complex" components) for which the css prop doesn't really work since there are numerous elements internally.

Imagine I wanted to build something like Ant's component library, which contains dozens of complex components, and one of the project requirements is to allow developers to have full control over re-styling internals–normally a trivial task with global css. Surprisingly, most css-in-js libraries simply don't provide a great story around this use case currently (I can give other examples), even though contextual style overrides for complex reusable components are an inevitability in large applications.

While I think this is a more important use case than many libraries are accounting for, I'm excited to see that there is a solution with Glamorous. Prominently displaying the "Accept style overrides" example in the docs looks like a good solution for now, and if there is more demand down the road then a helper package or core feature could be re-evaluated.

@kentcdodds
Copy link
Contributor

Actually, I think there's a much better way to design your API for these components that not only makes them more flexible from a styled standpoint, but from pretty much every angle. I'm working on an autocomplete component that uses this design pattern and will definitely share when I'm done. Spoiler alert, it looks similar to this:

ddchnuvvoaaljsb

That would be trivial to style all components also to use to build a more traditional (options based) API on top of if needed.

@schnerd
Copy link
Author

schnerd commented Jul 2, 2017

I've used the paradigm before where the reusable component internals are also exported (assuming that's what you're going for), and while it certainly allows for flexibility, in my opinion it makes the public interface too complicated. 99% of the time I'm using an autocomplete, a single-component API is sufficient and preferable (like react-select). Additionally, it's initially unclear whether child order matters–can I put a AutoComplete.Error before AutoComplete.Menu? This might also be really complicated for a datepicker–imagine rendering the weeks and days with a selected time range.

Certainly a good solution for some cases, but not sure it obviates the need for a styling-specific API.

@kentcdodds
Copy link
Contributor

Understood. You could definitely take the API described above and create a wrapper to expose a react-select like (options based) API in addition to the component pieces API (which I think is pretty great). But at that point, you're left with the same problem.

I agree that the solution in my example is probably the way to do this for now. I'm not super excited about baking in more complexity to handle this use case, but I would be happy to get this put together in a small library or something so using it takes less work (thus encouraging the pattern). Would you be willing to build that?

@schnerd
Copy link
Author

schnerd commented Jul 2, 2017

Yup, I'm up for that! Let me experiment with the helper function in one of our projects, and once it's been battle-tested I'll put together a small lib in a separate repo and solicit your feedback before publishing it. Thanks for your help and consideration on this 👍

Edit: After further experimentation, the "accept style overrides" solution started to break down for my particular use case (deep nesting of reusable components). I'm probably just going to drop back to using class names as described in #183

@taion
Copy link

taion commented Jul 2, 2017

Ah! I remember having this conversation with @threepointone on Twitter now. The context-based API is pretty neat, especially with syntactic sugar around it. It mostly works, which is neat. The one limitation was that (glancing through the docs in a cursory way) it doesn't really allow using more complex relational selectors, e.g. something like:

.form {
  & .btn + .btn {
    /* something */
  }

  & .input + .btn {
    /* something else */
  }
}

It's not a pattern we used a lot, but we had a few cases where we did a bit of that. Ended up using global hook classes with CSS Modules to get this working (& :global(.__button-hook) + :global(.__button-hook)), which was good enough.


FWIW, the appropriateness of that more complex API above depends a bit on the specific example. If e.g. your application uses autocompletes all over the place, it's probably a bit verbose to do that everywhere, but e.g. listing out <Tab> elements under a higher-level <Tabs> feels pretty natural to me. Does something like a :first-child selector work well in that context? Seems like "sort of"?

@kentcdodds
Copy link
Contributor

it doesn't really allow using more complex relational selectors, e.g. something like...

glamor (and hence glamorous) definitely supports selectors like that 👍

If e.g. your application uses autocompletes all over the place, it's probably a bit verbose to do that everywhere

The nice part of that kind of API is you could easily wrap it in another component and use that options-based API everywhere (while still allowing for the more powerful API).

@kentcdodds
Copy link
Contributor

Oh, and:

Does something like a :first-child selector work well in that context? Seems like "sort of"?

Definitely. You could try it out in https://help.glamorous.rocks and send along a link if you have any trouble.

@taion
Copy link

taion commented Jul 3, 2017

It's not a question of whether relational selectors are supported. It's a question of which relational selectors are supported. I've seen https://github.com/paypal/glamorous/#example-style-objects, which shows something similar to CSS modules across modularized classes – i.e.:

/* works */
.foo {
  & div + div {
    color: red;
  }
}

/* doesn't work */
.foo {
  & .btn + .btn {
    color: red;
  }
}

@kentcdodds
Copy link
Contributor

Ah, but that actually totally does work. Right now you'd have to manually add classes (which is trivial), but eventually we will have support for referring components in the selector directly.

But in either case, doing something like that is quite simple with glamorous

@taion
Copy link

taion commented Jul 3, 2017

Sure, so manually adding classes is about the same as what you get with CSS modules – global hook classes and all. It's okay, but ideally you'd want something within the system, e.g. explicit hooks (like the :external support that CSS modules will get, eventually).

That styled-components bit is really cool, though, and it's almost exactly what I was thinking.

@kentcdodds
Copy link
Contributor

If you'd like, glamorous's equivalent to that feature is ready for review: #183 😄

@kentcdodds
Copy link
Contributor

Closing in favor of: kentcdodds/glamorous-website#222

Thanks for the discussion everyone!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants