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

Variants working differently, __themeKey not exported #1247

Open
RobIsHere opened this issue Nov 9, 2020 · 10 comments
Open

Variants working differently, __themeKey not exported #1247

RobIsHere opened this issue Nov 9, 2020 · 10 comments

Comments

@RobIsHere
Copy link

Variants are working differently in different cases

import { css } from '@theme-ui/css';
css({ backgroundColor: 'textLink', fontFamily: 'body', height: 100, width: 100, variant: 'card.box' })

// Here the variant has different name, you need theme key (that is btw. missing in typescript definitions, so you can't build your own components in ts that use box like you do with buttons etc)
<Box ref={ref} variant="box" {...props} __themeKey="card"/>

But not only the naming is different. At the upper case variant is overwriting e.g. backgroundColor because its defined later.
At box a custom css would never be overwritten by the variant.

@hasparus
Copy link
Member

hasparus commented Nov 9, 2020

AFAIK __themeKey is not public API.

@RobIsHere
Copy link
Author

It's been a quite confusing journey for me to really get to know system-ui with variants. I don't know what to do about it, but maybe more documentation could help. I had the impression variants is documented on 3 Pages but nowhere really into depth. The way to use variants without the theme-ui components in theme-ui/css was missing.

The Problem with the privacy of __themeKey is that the whole system-ui/components package becomes useless in typescript because you cannot reuse and customize the components there. It simply is absurd to have to use the same theme keys on any derived components:

import React from 'react'
import Box from './Box'

export const MySuperFancyButton = React.forwardRef((props, ref) => (
  <Box
    ref={ref}
    as="button"
    variant="primary"
    {...props}
    __themeKey="mysuperfancybuttons"              /// <<< In typescript this doesn't compile!!! Error!
    __css={{
      appearance: 'none',
      display: 'inline-block',
      textAlign: 'center',
      lineHeight: 'inherit',
      textDecoration: 'none',
      fontSize: 'inherit',
      px: 3,
      py: 2,
      color: 'white',
      bg: 'primary',
      border: 0,
      borderRadius: 4,
    }}
  />
))

@atanasster
Copy link
Collaborator

I will add my vote that keeping __themeKey internal is making it harder than needed to create custom theme-ui components.
Also when converting theme-ui/componnets to typescript and using its own api - this will not work either.

@deckchairlabs
Copy link
Contributor

The components package could probably benefit from a useVariant hook.

import { useVariant, Button, PropsWithVariant } from 'theme-ui'

type AwesomeButtonProps = {
  somethingAwesome?: string
}

export const AwesomeButton = ({ variant, ...props }: PropsWithVariant<AwesomeButtonProps>) => {
  variant = useVariant('buttons', variant || 'awesome')
  return <Button sx={{...variant}} {...props} />
}

Ideally the internal API should be removed completely __css, __themeKey and just resort to using the hook for the base styles and any variants required.

@aaronadamsCA
Copy link
Contributor

I did this:

import type { BoxProps } from "theme-ui";
import { Box } from "theme-ui";

export const List = ({
  variant = "reset",
  ...props
}: BoxProps): JSX.Element => (
  <Box as="ul" variant={"lists." + variant} sx={{ my: 3 }} {...props} />
);

It feels like this shouldn't be necessary; but it's a simple enough workaround that I'm not too fussed about it.

@rafaelrinaldi
Copy link

@hasparus Hey, just wanted to bump this and ask if is there a better way to accomplish this rather than interpolating prefix and variant name manually (as suggested above).

@hasparus
Copy link
Member

hasparus commented Jan 7, 2022

Hey @rafaelrinaldi, is there anything you'd like to change Aaron's suggested solution? How would you like the API to look like?

@rafaelrinaldi
Copy link

@hasparus Thanks for the quick response. I am just trying to figure out what the consensus is on creating variants for custom components (non Theme UI). Is the way he suggested the way to go?

@lachlanjc
Copy link
Member

Indeed it is!

@rafaelrinaldi
Copy link

@lachlanjc Thank you!

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

No branches or pull requests

7 participants