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

A styled helper that works like classes #26685

Closed
1 task done
Jack-Works opened this issue Jun 10, 2021 · 5 comments
Closed
1 task done

A styled helper that works like classes #26685

Jack-Works opened this issue Jun 10, 2021 · 5 comments
Labels
out of scope The problem looks valid but we won't fix it (maybe we will revisit it in the future) package: styled-engine Specific to @mui/styled-engine

Comments

@Jack-Works
Copy link
Contributor

  • I have searched the issues of this repository and believe that this is not a duplicate.

Summary 💡

The main reason that blocking me to adopt styled component is that I have to write things like this:

const StyledDialogActions = styled(DialogActions)(({ theme }) => ({
    [`&.${dialogActionsClasses.root}>:not(:first-of-type)`]: {
        marginLeft: theme.spacing(3),
    },
    [`&.${dialogActionsClasses.spacing}`]: {
        // some style
    }
}))

It's would be better to have an API like this:

const StyledDialogActions = styled(DialogActions)({
    root: theme => {
        "&>:not(:first-of-type)": { marginLeft: theme.spacing(3) }
    },
    spacing: { }, // some style
    extra: {} // Type Error!
})

Motivation 🔦

It's cleaner and much type-safe. Please consider this.

Here is my idea in TypeScript

import { Dialog, Theme } from '@material-ui/core'

declare function styled<T, Q extends object>(
    comp: React.ComponentType<T & { classes?: Partial<Q> }>
): (styles: Record<keyof Q, React.CSSProperties | ((theme: Theme) => React.CSSProperties)>) => React.ComponentType<T>

const StyledDialog = styled(Dialog)({
    root: {
        color: 'red',
    },
    container: (theme) => ({ marginTop: theme.spacing(1) }),
    //   Object literal may only specify known properties, and 'thisOne' does not exist in type 'Record<keyof DialogClasses, CSSProperties | ((theme: Theme) => CSSProperties)>'.
    // ts error 2345
    thisOne: {},
})
@Jack-Works Jack-Works added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jun 10, 2021
@nuanyang233
Copy link

Maybe this is better

const StyledDialog = styled(Dialog)((theme, ...props) => {
    root: {
        color: 'red',
    },
    container: {
        marginTop: props.nested ? theme.spacing(1): 0
    }
})

@vicasas
Copy link
Member

vicasas commented Jun 14, 2021

This could have something to do with it, recently this RFC was opened to add support to the makeStyles api #26571

@mnajdova
Copy link
Member

We try to keep the styled() API identical (or at least extending) the emotion & styled-components' signature.

@eps1lon eps1lon added new feature New feature or request package: styled-engine Specific to @mui/styled-engine and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jul 13, 2021
@siriwatknp
Copy link
Member

I don't think we will spend time on this. Also, it should rely on CSS (.class) to overriding the styles, otherwise the API surface will be huge which have high maintenance cost in the future.

@siriwatknp siriwatknp added out of scope The problem looks valid but we won't fix it (maybe we will revisit it in the future) and removed new feature New feature or request labels Dec 20, 2024
Copy link

This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue.
Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

Note

@Jack-Works How did we do? Your experience with our support team matters to us. If you have a moment, please share your thoughts in this short Support Satisfaction survey.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
out of scope The problem looks valid but we won't fix it (maybe we will revisit it in the future) package: styled-engine Specific to @mui/styled-engine
Projects
None yet
Development

No branches or pull requests

6 participants