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

theme usage with darken from polished #52

Closed
DevanB opened this issue Oct 17, 2018 · 11 comments
Closed

theme usage with darken from polished #52

DevanB opened this issue Oct 17, 2018 · 11 comments

Comments

@DevanB
Copy link

DevanB commented Oct 17, 2018

Hey there!

Thanks for a great package full of lovely tools!

I'm running into an issue I don't quite understand:

background: ${darken(0.1, theme("green", "red")};

causes a TypeScript error (in my TS codebase):

[ts] Argument of type '<Props, Theme extends { [key: string]: any; }>(props: Props & { theme: Theme; }) => string | Theme[keyof Theme]' is not assignable to parameter of type 'string'.

The best I can figure out is that darkens types (darken(amount: string | number, color: string): string) expect a string, but theme isn't necessarily giving a string (it could be giving Theme[keyof Theme]). Would you have any understanding of how to solve this problem?

Thanks!

image

@diegohaz
Copy link
Owner

Hey @DevanB

Try this:

background: ${withProp(theme("green", "red"), darken(0.1))};

More info: https://github.com/diegohaz/styled-tools#withprop

@DevanB
Copy link
Author

DevanB commented Oct 17, 2018

This seems to work!

So withProp(theme('green')) is replacing ${props => something(props.theme.green)}?

Also, we are evaluating your tools (which look very promising!) for use throughout our component library. Does something like this look like a good refactor? Or am I missing some great other usages of your library? 😄

image

@diegohaz
Copy link
Owner

diegohaz commented Oct 17, 2018

So withProp(theme('green')) is replacing ${props => something(props.theme.green)}?

Exactly! 😉

I'd write this way:

const background = switchProp("color", {
  green: withProp(theme("green"), darken(0.1)),
  ink: withProp(theme("ink"), darken(0.15))
});

console.log(background({ color: "green", theme: { green: "green" } }));

const Comp = styled.div`
  background: ${background};
`;

Or, if the only variable thing were the prop value:

const background = withProp("color", color => withProp(theme(color), darken(0.1)));

This can be composed in different ways. Just be careful not to abstract to the point of reducing its legibility (splitting large compositions into well named functions helps).

Basically, all the functions return another function that receive props, and you can recursively call them.

@diegohaz
Copy link
Owner

Ah, and note that we're leveraging the curried version of polished function (you can use darken(arg1)(arg2))

@DevanB
Copy link
Author

DevanB commented Oct 17, 2018

@diegohaz Thanks much!! This is super valuable!

@DevanB DevanB closed this as completed Oct 17, 2018
@DevanB
Copy link
Author

DevanB commented Oct 18, 2018

As a followup, I was able to turn:

const background = switchProp('color', {
  green: theme('green'),
  ink: theme('ink'),
  red: theme('red'),
  white: theme('white'),
});

background: ${background};

into:

background: ${withProp('color', incomingColor => theme(incomingColor))};

@diegohaz
Copy link
Owner

In that case, I guess this works:

withProp('color', theme)

@DevanB
Copy link
Author

DevanB commented Oct 18, 2018

Fantastic! I'm going to make it a point to submit a PR for your documentation. For people like me that stumble upon this hidden gem, it might be good to have a little "recipes" section that shows basic and highly usable examples!

@diegohaz
Copy link
Owner

That would be really useful. Thank you! :)

@DevanB
Copy link
Author

DevanB commented Oct 19, 2018

@diegohaz one last question! Do you see any reason why this:

const hoverFocusBackground = switchProp('color', {
  green: withProp(theme('green'), darken(0.1)),
  ink: withProp(theme('ink'), lighten(0.15)),
  red: withProp(theme('red'), darken(0.1)),
  white: withProp(theme('white'), darken(0.1)),
});

would still throw an error of:

<Button /> › matches snapshot

Passed an incorrect argument to a color function, please pass a string representation of a color.

       8 | describe('<Button />', async () => {
       9 |   it('matches snapshot', () => {
    > 10 |     const { container } = render(<Button color="red">Test</Button>);
         |                           ^
      11 |     expect(container.firstChild).toMatchSnapshot();
      12 |   });
      13 | });

      at parseToRgb (node_modules/polished/lib/color/parseToRgb.js:42:11)
      at parseToHsl (node_modules/polished/lib/color/parseToHsl.js:34:57)
      at darken (node_modules/polished/lib/color/darken.js:58:42)

in my Jest snapshot tests?

The test is a basic snapshot test:

import React from 'react';
import { render } from 'react-testing-library';
import { Button } from '../Buttons';

describe('<Button />', async () => {
  it('matches snapshot', () => {
    const { container } = render(<Button color="red">Test</Button>);
    expect(container.firstChild).toMatchSnapshot();
  });
});

I wouldn't think something like this is affecting me: https://stackoverflow.com/questions/47704601/styled-components-polished-and-styledprops-darken-throwing-error

The best I can tell, you are returning a string when currying.

I created a CodeSandbox showing the issue here: https://codesandbox.io/s/18834qr52l

@DevanB DevanB reopened this Oct 19, 2018
@DevanB
Copy link
Author

DevanB commented Oct 19, 2018

I'm so sorry for bothering. Figured out the problem was my tests didn't have access to theme because I wasn't 1) passing it in, nor 2) wrapping the test in ThemeProvider.

Thanks for dealing with me 😄

@DevanB DevanB closed this as completed Oct 19, 2018
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

No branches or pull requests

2 participants