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

CSS Custom Props Have Spaces #123

Closed
dev-nicolaos opened this issue Sep 27, 2023 · 4 comments · Fixed by #131
Closed

CSS Custom Props Have Spaces #123

dev-nicolaos opened this issue Sep 27, 2023 · 4 comments · Fixed by #131
Labels
bug Something isn't working

Comments

@dev-nicolaos
Copy link
Contributor

dev-nicolaos commented Sep 27, 2023

It's possible for token or group object keys to contain a space. To the best of my knowledge, this is not disallowed by the spec (though specific cases like the empty string and leading/trailing white space are being considered).

Those space don't currently cause any issues (that I can tell) with Cobalt parsing the tokens, and the rendered output by the JS plugin is valid, using strings as keys. But they become a problem when trying to use the CSS plugin to produce custom properties. CSS custom properties cannot have spaces in them, but Cobalt renders the spaces as is anyway creating invalid CSS.

Example

Tested with

  • "@cobalt-ui/cli": "^1.6.0",
  • "@cobalt-ui/plugin-css": "^1.5.0",
  • "@cobalt-ui/plugin-js": "^1.4.1"

Given the following tokens input

{
  "Full Color Palette": {
    "Magenta": {
      "Magenta-50": {
        "$type": "color",
        "$value": "#fff0f6",
        "$description": ""
      },
      "Magenta-100": {
        "$type": "color",
        "$value": "#ffd6e7",
        "$description": ""
      }
    }
  }
}

Running npx co build produces the following output:

A valid `index.js` file
/**
 * Design Tokens
 * Autogenerated from tokens.json.
 * DO NOT EDIT!
 */

export const tokens = {
  'Full Color Palette.Magenta.Magenta-50': '#fff0f6',
  'Full Color Palette.Magenta.Magenta-100': '#ffd6e7',
};

export const meta = {
  'Full Color Palette.Magenta.Magenta-50': {
    _original: {
      $type: 'color',
      $value: '#fff0f6',
      $description: '',
    },
    _group: {
      id: 'Full Color Palette.Magenta',
      $extensions: {
        requiredModes: [],
      },
    },
    id: 'Full Color Palette.Magenta.Magenta-50',
    $type: 'color',
    $value: '#fff0f6',
    $description: '',
  },
  'Full Color Palette.Magenta.Magenta-100': {
    _original: {
      $type: 'color',
      $value: '#ffd6e7',
      $description: '',
    },
    _group: {
      id: 'Full Color Palette.Magenta',
      $extensions: {
        requiredModes: [],
      },
    },
    id: 'Full Color Palette.Magenta.Magenta-100',
    $type: 'color',
    $value: '#ffd6e7',
    $description: '',
  },
};

export const modes = {};

/** Get individual token */
export function token(tokenID, modeName) {
  if (modeName && modes[tokenID] && modeName in modes[tokenID]) return modes[tokenID][modeName];
  return tokens[tokenID];
}
An invalid tokens.css file
/**
 * Design Tokens
 * Autogenerated from tokens.json.
 * DO NOT EDIT!
 */

:root {
  --Full Color Palette-Magenta-Magenta-50: #fff0f6;
  --Full Color Palette-Magenta-Magenta-100: #ffd6e7;
}

@supports (color: color(display-p3 1 1 1)) {
  :root {
    --Full Color Palette-Magenta-Magenta-50: color(display-p3 1 0.9411764705882353 0.9647058823529412);
    --Full Color Palette-Magenta-Magenta-100: color(display-p3 1 0.8392156862745098 0.9058823529411765);
  }
}

Options

  1. This a known and expected limitation of the CSS plugin
    • If so, it would be a good idea to document it since this isn't an issue with other formats.
  2. The plugin replaces any spaces in a key with a - the same way it does in between group and token names
  3. The plugin replaces any spaces in a key with an _ to distinguish between spaces within a name and the split between different token and group names
  4. The developer is given the ability to handle this in the plugin's config. Maybe an expansion of the transform option to allow editing more than the value, or another option all together.

I think option 3 is my personal preference, but I don't have a strong opinion. Just hoping this can get resolved quickly.

Thanks for your work on this tool! Looking forward to seeing the design tokens space mature.

@drwpow drwpow added the bug Something isn't working label Sep 28, 2023
@kompolom
Copy link

kompolom commented Oct 5, 2023

Maybe we can separate groups with -- and replace spaces with -.
Something like:

:root {
 --Full-Color-Palette--Magenta--Magenta-50: #fff0f6;
}

@dev-nicolaos
Copy link
Contributor Author

I've been thinking about this a bit and I think the best solution is having an option in the plugin's config where the user can specify a string to replaces spaces in names. That option can have a default value ("_" makes the most sense to me but I don't have a strong preference) so it works out of the box.

Maybe we can separate groups with -- and replace spaces with -.

If the approach I described above works, then it probably wouldn't be to hard to add an additional option for separating groups. Personally I feel like that should be different issue since it isn't breaking anything now, whereas the spaces in names issue is a bug .

@drwpow
Copy link
Collaborator

drwpow commented Oct 13, 2023

I agree—letting the user control the name generation is probably best (but in addition to the core issue of spaces being fixed). Will get to this soon, but I’d also accept PRs in case someone beats me to it 🙂

@dev-nicolaos
Copy link
Contributor Author

I'll see if I can spend some time working on this in the next couple days

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants