X Tutup
The Wayback Machine - https://web.archive.org/web/20201020185330/https://github.com/amzn/style-dictionary/issues/307
Skip to content

/ style-dictionary

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

Jump to bottom

Sass variables should (optionally?) have `!default` flag #307

Open
nategreen opened this issue Jul 8, 2019 · 4 comments
Open

Sass variables should (optionally?) have `!default` flag #307

nategreen opened this issue Jul 8, 2019 · 4 comments
Labels
format good first issue hacktoberfest help wanted
Projects
SD 3.0
Milestone
3.0

Comments

@nategreen
Copy link

@nategreen nategreen commented Jul 8, 2019

In the scss/map-deep format, the variables it generates (for use in the map) have the !default flag:

// '!default' means if the variable is/was already defined, this declaration gets skipped
$my-var: #123ABC !default; 
$tokens: (
  my: (
    var: $my-var,
  )
);

…whereas the scss/variables format doesn't have that:

// No !default, so this variable can't be overridden during Sass compilation
$my-var: #123ABC;

Is there a reason why one has it and the other doesn't? To me, it seems like:

  • These should agree…It's not necessarily a huge deal, but currently if we build both formats, downstream Sass compilations will either be able to override the values or not, based on which format they choose.
  • !default should be added as an option so we can choose whether or not we want them to be able to override the variables from the configuration. (I guess that's a feature request. 😄)
@dbanksdesign
Copy link
Member

@dbanksdesign dbanksdesign commented Jul 8, 2019

I like having !default as an optional configuration. Do you think it would make sense to do that at the file level or at the token level?
@nategreen
Copy link
Author

@nategreen nategreen commented Jul 9, 2019

I was just picturing it being an option on the format in the config—beyond that, not sure. I guess the token level makes sense for overrides, not sure about file level…are there other options/features that work at the file level?
@nategreen
Copy link
Author

@nategreen nategreen commented Jul 9, 2019

so we can choose whether or not we want them to be able to override the variables from the configuration

Oh, I think I was multi-tasking when I wrote this, I just meant "so we can choose whether or not downstream users can override the variables," full stop. I don't know if I can conjure a reason we'd want the default flag on some tokens but not others, but it doesn't seem out of the question.
jmmcduffie added a commit to RamseyInHouse/style-dictionary that referenced this issue Nov 23, 2019
@jmmcduffie
feat: add support for !default in SCSS variables format
f0e837e 
For any token definition that includes the property
`themeable: true`, a `!default` will now be added in
the scss/variables format.

GitHub issue amzn#307
@jmmcduffie jmmcduffie mentioned this issue Nov 23, 2019
Merged
jmmcduffie added a commit to RamseyInHouse/style-dictionary that referenced this issue Nov 25, 2019
@jmmcduffie
feat: add support for !default in SCSS variables format
12b7754 
For any token definition that includes the property
`themeable: true`, a `!default` will now be added in
the scss/variables format.

GitHub issue amzn#307
@chazzmoney chazzmoney added this to the 3.0 milestone Oct 8, 2020
@chazzmoney chazzmoney added this to To do in SD 3.0 via automation Oct 8, 2020
dbanksdesign pushed a commit that referenced this issue Oct 8, 2020
@jmmcduffie
feat: add support for !default in SCSS variables format (#359)
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
fa82002 
Fixes #307

For any token definition that includes the property
`themeable: true`, a `!default` will now be added in
the scss/variables format.
@dbanksdesign
Copy link
Member

@dbanksdesign dbanksdesign commented Oct 8, 2020

We merged in #359 onto the 3.0 branch targeting the next major release. We will keep this issue open until the release. We want to see if this flag themeable: true on the token can be used in other built-in formats so that this is not SCSS specific. If anyone has ideas for this please open a PR or comment on this issue: #474 thanks!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
format good first issue hacktoberfest help wanted
Projects
SD 3.0
  
To do
Milestone
3.0
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
@dbanksdesign @chazzmoney @nategreen
You can’t perform that action at this time.
X Tutup