-
Notifications
You must be signed in to change notification settings - Fork 60
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
Adds new token property ($private
)
#172
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This effectively makes this a write-only storage.
Once someone marks a token as private it would become invisible to them because their tool would need to honour the flag.
It might be more interesting to have "filter" tools that teams can use to convert one or more "source" token files into a "filtered" final token file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 Tools could be required to have a setting to show or hide
private
tokens.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It depends on how you see these files being used. A token editor tool, could consume the input JSON and would be able to see all tokens and allow you to see/modify the private flag.
A translator on the other hand, would resolve private tokens, but would not export them to any output files (e.g. CSS, XML, JSON etc.)
I feel designers and developers should be consuming a processed output and see the input JSON as the source of truth, which only a certain set of designers modify. This could be an alternative approach to filter tools.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this wording would need to change to be clearer around different types of tooling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it then not be better to have a token management app/service which stores this information under
$extension
. Team members would use this app/service to configure/compose different exports for certain other teams to consume.I don't think this requires a spec change.
I don't think it makes sense to encode a hierarchy of control over tokens in the spec. (what if tokens are created by developers with dev tools in a certain org?)
I do think it can be useful to have a filter to reduce clutter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also see #110
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue that I'm facing is that I want to use tokens to create tokens. However, I don't want the tokens I used to create the tokens to be pomoted for use - They are used for computing other tokens, and I want to pass this information to translators so that they can choose how to handle them. An option could be passed to a translator to include them in the output. It's a way to structure
tokens.json
and inform translators which tokens are relevant and should be exported by default. It may only be a single token that is marked as private, but is used to create many other token values.The example in the description is probably not the best. It hides colors from designers, which is not ideal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$hidden
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Classes in certain programming languages also have to concepts of
abstract
andfinal
. I think that has some overlap.abstract
-> only intended to be used to construct other tokensfinal
-> never intended to be used to construct other tokensTokens with
$abstract
:Tokens with
$final
:You don't need both concepts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After thinking about this a bit more, I don't think we need to go that far, we're not designing a new programming language here - it's essentially a configuration file.
$abstract
and$final
feel a bit too much.I agree that
$private
is the wrong name, however, and I would suggest it to be more like a visibility state, hidden/visible. Token editing tools would be able to see all tokens and modify the visibility state for translators.$hidden
seems like a good option.