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

textStyles defined inside extendTheme aren’t applied correctly #3884

Open
rastersysteme opened this issue Apr 22, 2021 · 16 comments
Open

textStyles defined inside extendTheme aren’t applied correctly #3884

rastersysteme opened this issue Apr 22, 2021 · 16 comments

Comments

@rastersysteme
Copy link

🐛 Bug report

Styles defined via textStyles are not applied correctly although they’re set within extendTheme.

💥 Steps to reproduce

  1. Go to my CodeSandbox
  2. Inspect Heading, Button, Text and the body element with the developer tools inside the browser
  3. Have a look at the styles set by Chakra UI and compare them

textStyles which should be applied

image

body styles

image
font-size from textStyles is used.
💥 font-family and line-height aren’t applied by textStyles.

Heading styles

image
💥 Neither font-family nor font-size and line-height are applied by textStyles.

Button styles

image
font-family from textStyles is used.
💥 font-size and line-height aren’t applied by textStyles.

Text styles

image
✅ All styles set by textStyles are used.

💻 Link to reproduction

CodeSandbox reproduction: https://codesandbox.io/s/textstyle-problems-ibu9h?file=/src/App.tsx:176-186

🧐 Expected behavior

Styles like font-family, font-size and line-height defined in textStyles should be applied when passed through extendTheme according to this comment. This behaviour should be consistent and reliable when used on all elements (even <body />).

🧭 Possible Solution

🌍 System information

Software Version(s)
Chakra UI 1.5.2
Browser Chrome, Firefox
Operating System macOS

📝 Additional information

@stale
Copy link

stale bot commented Jun 2, 2021

Hi!
This issue hasn't seen any activity recently. We close inactive issues after 35 days to manage the volume of issues we receive.
If we missed this issue or you want to keep it open, please reply here. That will reset the timer and allow more time for this issue to be addressed before it is closed.

@stale stale bot added the stale label Jun 2, 2021
@tbaschim
Copy link

tbaschim commented Jun 2, 2021 via email

@stale stale bot removed the stale label Jun 2, 2021
@bangngo1508
Copy link

bangngo1508 commented Jun 8, 2021

I am facing the same issue with the defaultProps

image

image

The defaultProps is not applied at all

@rastersysteme
Copy link
Author

Tested it with the latest version 1.6.3, still the same problem. Sadly, at the moment textStyles aren’t usable.

@crimson-med
Copy link

crimson-med commented Jun 24, 2021

Any updates on this?
I'm trying to customize StatHelperText and it is working on old version 1.1.2 of chakra but the recent 1.6.4 not working.

<StatHelpText textStyle="h1" >
    <StatArrow type="increase" /> 10%
</StatHelpText>

Variant and defaultProps with StatHelpText, StatNumber are also not working

@rastersysteme
Copy link
Author

Bump – I really would love to see a solution for this.

@stephane-ruhlmann
Copy link

stephane-ruhlmann commented Jul 20, 2021

Same, can't manage to have fontSize defined in textStyle work with <Heading/> since I updated to 1.6.5.
Worked fine with version 1.1.6.

Here's a narrowed down example in CodeSandbox

color appears to be applied but not fontSize

textStyles are defined as:

const customTheme = extendTheme({
  textStyles: {
    h1: {
      fontSize: "32px",
      color: "blue",
    },
    h2: {
      fontSize: "24px",
      color: "red",
    },
    h3: {
      fontSize: "16px",
      color: "orange",
    }
  }
});

And used this way:

<VStack spacing="8px">
      <Heading as="h1" textStyle="h1">
        Heading 1
      </Heading>
      <Heading as="h2" textStyle="h2">
        Heading 2
      </Heading>
      <Heading as="h3" textStyle="h3">
        Heading 3
      </Heading>
</VStack>

@stephane-ruhlmann
Copy link

After some further investigation it looks like we should be using theme variants for components such as <Heading/> as suggested there #3501 (comment)

@rastersysteme
Copy link
Author

After some further investigation it looks like we should be using theme variants for components such as <Heading/> as suggested there #3501 (comment)

Sadly this doesn’t tackle the base problem – just have a look for example at my initial sandbox where I applied textStyle on body. And even inside the variants it won’t work.

To cite the documentation:

The text styles functionality in Chakra makes it easy to repeatably apply a collection of text properties (like line height and size) to any component.

So as described I just want to be able to apply my pre-definied textStyle definitions (which are all saved in one single file) to any component as I did prior to V1.4. This isn’t possible at the moment.

@stephane-ruhlmann
Copy link

stephane-ruhlmann commented Jul 20, 2021

After some further investigation it looks like we should be using theme variants for components such as <Heading/> as suggested there #3501 (comment)

Sadly this doesn’t tackle the base problem – just have a look for example at my initial sandbox where I applied textStyle on body. And even inside the variants it won’t work.

To cite the documentation:

The text styles functionality in Chakra makes it easy to repeatably apply a collection of text properties (like line height and size) to any component.

So as described I just want to be able to apply my pre-definied textStyle definitions (which are all saved in one single file) to any component as I did prior to V1.4. This isn’t possible at the moment.

Yes, this is definitely a bug as it should work as stated in the documentation. Should have made it clear in my prior comment.

The suggestion is a workaround that might not work for everywhere but as it appears to be a complex issue on chakra's side that's all we can do for now. I hope they can fix it in an upcoming release as it's not a satisfying solution.

@primos63
Copy link
Contributor

primos63 commented Aug 25, 2021

This issue occurs with both text and layer styles. It's possible it occurs with apply also. The reason for this is priority is currently given to attributes which exist in the theme. For Text it works because there is no theme source for it. Heading has all three attributes defined in it's theme source and Button has lineHeight and fontSize. body has fontFamily and lineHeight defined in the default global theme setting (packages/theme/src/style.ts).

I'm not sure why priority was given to the theme. It would seem that what is specified in the JSX would have a higher precedence. If you were to specify the attributes separately in JSX, <Heading textStyle="h1" fontSize={128}>...</Heading>, fontSize takes precedence over what's defined in the theme for Heading.

I can remove the style lookup, but I'm not sure what the thinking was at the time. There are no comments in the code nor is there anything in the documentation about theme styles having precedence over text and layer styles.

** Update **
I found the PR which caused this issue.

@ddkang
Copy link

ddkang commented Oct 13, 2021

Any word on this? It looks like there's an umerged PR aiming to fix this.

@heydoughenrique
Copy link

Guys, and if you adopt managing those text styles with global settings instead textStyles?

Instead of using:

<Box>
      <Heading as="h1" textStyle="h1">
        Heading 1
      </Heading>
</Box>

and

const customTheme = extendTheme({
  textStyles: {
    h1: {
      fontSize: "24px",
    },
  }
});

Try it:

<Box>
      <Heading as="h1">
        Heading 1
      </Heading>
</Box>
const customTheme = extendTheme({
    styles: {
        global: {   
            // textStyles
            h1: {
                fontSize: "24px",
            },
        }
    },
})

Share with me if it works.

Thank you!

@primos63
Copy link
Contributor

@stridesdigital That defeats the purpose of Text and Layer Styles. You don't get the same advantage using global styles. One can define a style with h1 having a font-size of 24px and another style where h1 has a font-size of 48px. Using global styles the h1 can only exist once.

@heydoughenrique
Copy link

@primos63 Preserving font style properties through your project makes it easier to maintain and make it more consistent.

But, if you really need to change a specific text style, instead of facing a Chakra UI bug (topic related) by using textStyle, you can customize your extended theme.

Instead of using this:

const customTheme = extendTheme({
    styles: {
        global: {   
            // textStyles
            h1: {
                fontSize: "24px",
            },
        }
    },
})

You can try this:

import { HeadingStyles as Heading } from './styles/heading'

export const customTheme = extendTheme({

    components: {
        Heading,
    },

    styles: {
        global: {
            // textStyles
            h1: {
                fontSize: Heading.sizes['2xl'].fontSize,
                fontWeight: Heading.variants.fontWeight.bold,
            },
        }
    },
})

Your HeadingStyles will look like that:

export const HeadingStyles = {

    baseStyle: {
        fontFamily: "heading",
        fontWeight: "",  // make sure it's empty
    },
    // styles for different sizes ("sm", "md", "lg")
    sizes: {
        "5xl": {
            fontSize: "128px",
        },
        "4xl": {
            fontSize: "96px",
        },
        "3xl": {
            fontSize: "72px",
        },
        "2xl": {
            fontSize: "60px",
        },
        "lg": {
            fontSize: "46px",
        },
        "md": {
            fontSize: "36px",
        },
        "sm": {
            fontSize: "30px",
        },
        "xs": {
            fontSize: "24px",
        },
    },
    // styles for different visual variants ("outline", "solid")
    variants: {
        fontWeight: {
            normal: 400,
            medium: 500,
            semibold: 600,
            bold: 700,
        }
    },

And finally, you can use for global style:

<Box>
      <Heading as="h1">
        Heading 1
      </Heading>
</Box>

And this for another customized value:

<Box>
      <Heading as="h1" size="5xl" fontWeight="normal">
        Customized Heading 1
      </Heading>
</Box>

What did you think? Does this provide any help?

@primos63
Copy link
Contributor

The issue was with the precedence order for which I believe I fixed and submitted a PR which has yet to be reviewed. I have no control over that. Workarounds should be temporary.

@primos63 primos63 self-assigned this Nov 9, 2021
@primos63 primos63 added the Type: Bug 🐛 Something isn't working label Nov 24, 2021
@primos63 primos63 removed their assignment Feb 2, 2022
@chakra-ui chakra-ui locked and limited conversation to collaborators Mar 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
9 participants