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

Provide a default tracking with Font Size config #1915

Merged
merged 1 commit into from
Aug 6, 2020

Conversation

valtism
Copy link
Contributor

@valtism valtism commented Jun 12, 2020

We can already configure line-height with font sizes, but this allows
us to also specify the default tracking for these sizes too.

This can be overwritten with tracking utilities.

I asked here but got no response: #1874

@adamwathan
Copy link
Member

I like this idea a lot! My only hesitation is that bumping this up to 3 unnamed positional parameters is getting into "not obvious" territory in terms of what things do. I think it's probably fine but I wonder if it would be worth introducing an object syntax at this point so things are more clear, like:

module.exports = {
  theme: {
    // ...
    fontSize: {
      sm: ['14px', { lineHeight: '20px', letterSpacing: '0.01em' }],
      // ...
    }
  }
}

What do you think, is it too verbose for the people who would want this feature or is it worth it for the extra clarity?

@estevanmaito
Copy link
Contributor

I would vote for clarity in 10/10 cases. Better for documenting, even better because somebody wouldn't even need to look into the docs to know what the code above is doing.

@valtism
Copy link
Contributor Author

valtism commented Jun 28, 2020

Sounds good. The one issue I see is that we will have to support both the current 2-parameter syntax and this new object syntax, which might be confusing in the documentation.

Extend the configuration syntax to allow specifying letterSpacing
alongside lineHeight in the fontSize config.

The syntax looks like
```
module.exports = {
  theme: {
    // ...
    fontSize: {
      sm: ['14px', { lineHeight: '20px', letterSpacing: '0.01em' }],
      // ...
    }
  }
}
```

Also retains backwards compatibility with array syntax of 1.3
@valtism valtism reopened this Jun 29, 2020
@valtism
Copy link
Contributor Author

valtism commented Jun 29, 2020

Changed to the suggested syntax

@Larsklopstra
Copy link

Just wanted to make a PR and came across this, can't wait for it to be released 🎉

Copy link

@ilyosjon09 ilyosjon09 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

u to

src/plugins/fontSize.js Show resolved Hide resolved
@valtism
Copy link
Contributor Author

valtism commented Jul 23, 2020

@adamwathan Sorry to bother you, but are there plans for this to be included in any upcoming release?

@adamwathan adamwathan merged commit 2c05a4a into tailwindlabs:master Aug 6, 2020
@valtism
Copy link
Contributor Author

valtism commented Aug 19, 2020

@Larsklopstra Pinging you to let you know this has been released

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

Successfully merging this pull request may close these issues.

5 participants