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

Solution for composition? #2

Closed
awdyson opened this issue Apr 19, 2021 · 6 comments
Closed

Solution for composition? #2

awdyson opened this issue Apr 19, 2021 · 6 comments

Comments

@awdyson
Copy link

awdyson commented Apr 19, 2021

Any way we can reuse styles that rely on props/state?

const { useClassNames } = createUseClassNames()((props) => ({
  link: {
    ...
    '&:hover':  {
      extend: 'activeLink', // JSS solution
    },
  },
  activeLink: {
    // some CSS that relies on props
  },
}));
@garronej
Copy link
Owner

garronej commented Apr 20, 2021

Sorry for late reply @awdyson ,
Sure you can it's a very important requirement!

type Props ={
    size: "big" | "small";
};

const { useClassNames } = createUseClassNames<Props & { color: "red" | "blue" }>()(
    (theme, { size, color }) => ({
        link: {
        ...
        '&:hover':  {
          extend: 'activeLink', // JSS solution
        },
      },
      activeLink: {
          // Here you have access to "size" from Props and "color" from internal state 
      },
}));


function MyComponent(props: Props){

    const [ color, setColor ] = useState<"red" | "blue">("red");

    const { classNames } = useClassNames({ ...props, color });


   //...

}

Best regard

@awdyson
Copy link
Author

awdyson commented Apr 21, 2021

Sorry, I should have been more clear.

I'm looking to share styling between classes, within the context of a single createUseClassNames call.
In the example above, I want a link's :hover state to look the same as its react router activeClassName state.
With JSS, there exists the extend plugin: https://cssinjs.org/jss-plugin-extend/
The best plan I've come up with for this would be an external function that's called with theme/props.

So, to rework the example above:

const helper = (theme, props) => ({
  // shared style that relies on theme and/or props
});

const { useClassNames } = createUseClassNames<Props>()((theme, props) => ({
  link: {
    // other styles
    '&:hover':  helper(theme, props),
  },
  activeLink: helper(theme, props),
}));

Hopefully this makes sense.
Also, this is definitely more of a nice to have, considering there's a workaround.
I figured it was in-line with your projects goals though.

@garronej
Copy link
Owner

@awdyson This is actually a very good point, we need to support that.
Very easy to implement but hard to type.
I will try to think of something...

@garronej
Copy link
Owner

garronej commented Jul 7, 2021

I will, soon, implement something like that:

const { classes, getLinkCss } = makeStyles<Props>()(
    (theme, props) => ({
        "link": {
            // other styles
            "&:hover":  {
               //style specific to hover link
           },
        },
        "activeLink": {
           ...getLinkCss(theme, props)["&:hover"],
           //style specific to activeLink
        }
    })
);

//getLinkCss can be exported for reuse outside of the file.
export { getLinkCss };

We'll get correct typing of getLinkCss, getActiveLinkCss using template literal types.

There will be some voodoo involved to prevent infinite recursion behind the seen but it will be transparent to the user.

@garronej
Copy link
Owner

In the end I figured out that there is no need for a dedicated API.
Composition documentation

@awdyson
Copy link
Author

awdyson commented Jul 26, 2021

Haven't been able to work on this project in a while, so I'm catching up on a lot of updates.

Your solution looks great -- and is the sort of thing I couldn't do well in JSS+MUI for a reason I can't remember.
Also, the method and property names look a lot cleaner these days.
Appreciate the great work 😃

gitbook-com bot pushed a commit that referenced this issue Jan 22, 2022
gitbook-com bot pushed a commit that referenced this issue Jul 10, 2022
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