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.

Sign up for GitHub

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

add alias option to typescale #68

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

add alias option to typescale #68

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
module.exports = {
"typeScale": [
3, 2.25, 1.5, 1.25, 1, 0.875, 0.75
{value: 3, alias: "title"},
{value: 2.25, alias: "sub"},
{value: 1.5, alias: ["copy", "error"]},
1.25, 1, 0.875, 0.75
],
"spacing": {
"root": 8,
Expand Down
3 changes: 2 additions & 1 deletion lib/docs-helper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@ const suffix = (idx) => {
}
}

function step({ zeroth = '', nth = '' }, idx) {
function step({ zeroth = '', nth = '' }, idx, alias) {
if (idx === 0) { return `0 = 0${zeroth}` }
const last = Math.abs(idx).toString().split('').reverse()[0]

if (alias) return `${idx} = ${alias}`
return `${idx} = ${idx}${suffix(last)} step in ${nth}`
}

Expand Down
14 changes: 12 additions & 2 deletions lib/type-scale.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const { step, mqSteps } = require('./docs-helper')
const isObject = require('isobject')

const docs = (steps, mqs) => `
/*
Expand All @@ -9,7 +10,7 @@ const docs = (steps, mqs) => `
f = font-size

Modifiers:
${ steps.map((_, idx) => step({ nth: 'size scale' }, idx + 1)).join('\n ') }
${ steps.map((s, idx) => step({ nth: 'size scale' }, idx + 1, s.alias)).join('\n ') }

Media Query Extensions:
${mqSteps(mqs)}
Expand All @@ -25,7 +26,16 @@ const css = typeScale => {
return steps.join('\n')
}

const decl = (step, size) => `.f${step} { font-size: ${size}rem; }`
const decl = (step, size) => {

let classNames = [step]

if (isObject(size)) {
classNames = classNames.concat(size.alias)
size = size.value
}
return `.f${classNames.join(', .f')} { font-size: ${size}rem; }`
Copy link
Member

Choose a reason for hiding this comment

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

100% absolutely love this. Only thing I wonder about is if we make the alias not auto prefixed with f?

So:

{value: 1.5, alias: ["copy", "error"]}

Becomes

.f3, .title, .copy

And if you desire the f prefix you can add in the aliases: ["fcopy", "ferror"]. Might make it a touch more flexible. Thoughts?

Copy link
Author

Choose a reason for hiding this comment

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

this PR for typescale is just a proof of concept. if we implement this, it should be generally available for all scales (spacings, heights, widths, etc...).
so omitting the class prefix leads to 2 problems:

  1. we have to validate the aliases globally so the user doesn't override his aliases across different scales
  2. it will be harder to understand the classes when reading them in html code, because if it says class="error", you have no idea what exactly this class is defining. if it's class="ferror" you will always know (by tachyons naming conventions), this class has something todo with fonts

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, definitely agree it should be handled for all the things. I think we can look for identical aliases and warn when that occurs while also piping the output css through immutable-css to log any conflicting class names.

I'd like to see the ability to actually "eject" from the Tachyons naming conventions if they so choose, but that's prolly a bit different than aliasing. So perhaps aliases always are prefixed with the class base and one can actually change the class base when they desire something different:

{
  typeScale: {
    values: [1,2,3, { value: 4, alias: ['big', 'headline' }],
    classBase: 'heading'
  }
}

I'll do some thinking on this over the weekend, as it'd be great to come up with a config that can handle all of this while still maintaining a sensible config api.

Copy link
Author

Choose a reason for hiding this comment

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

also, I was wondering, why do you handle spacing different then the other settings?
https://github.com/tachyons-css/tachyons-generator/blob/master/lib/spacing.js#L54

with this step/ratio setting, we wouldn't be able to apply this proposal

Copy link
Member

Choose a reason for hiding this comment

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

I think that was a poor api choice on my behalf. I think we should prolly move it to become inline with everything else (which should be okay since we're not 1.0.0 yet).

}

module.exports = {
css,
Expand Down