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

Add layered shadow support, add support for inset shadows (non-breaking changes) #246

Merged
merged 1 commit into from
Aug 22, 2024

Conversation

drwpow
Copy link
Contributor

@drwpow drwpow commented Aug 17, 2024

Summary

This proposes the following changes to $type: shadow tokens:

  • Allows arrays for $value
  • Adds inset (boolean) property

Reasoning

This PR proposes that the following discussions be resolved like so:

Pros

  • Adds layered shadow support (example) without breaking existing shadow tokens
  • Adds support for inset shadows, also without breaking backwards compatibility

Cons

  • Arrays vs non-arrays adds a minor bit of tooling annoyance, but nothing significant

Alternatives

  • We could just enforce “always arrays,” but IMO that’s not necessary to put that burden on the people writing JSON. The spec allows for other “syntactic sugar” and this seems like a good QoL addition
  • We could make inset required, at the expense of breaking backward compatibility
  • Instead of "inset": [boolean], we could have something like "position": "inner" (string enum), but I can’t think of any other value this would be (what is there other than “inside” and “outside”?). Seems like designing for something no one has asked for.

Notes

  • Other token types such as gradients and cubic béziers already have arrays for values, so this doesn’t introduce inconsistencies in the design

Copy link

netlify bot commented Aug 17, 2024

Deploy Preview for dtcg-tr ready!

Name Link
🔨 Latest commit e4f734a
🔍 Latest deploy log https://app.netlify.com/sites/dtcg-tr/deploys/66c118176c3ed00008fbeb9e
😎 Deploy Preview https://deploy-preview-246--dtcg-tr.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@drwpow drwpow mentioned this pull request Aug 17, 2024
@kaelig
Copy link
Member

kaelig commented Aug 22, 2024

This proposal looks good to me, we'll make adjustments to the color values separately shortly.

@kaelig kaelig changed the title Proposal: changes to $shadow Add layered shadow support, add support for inset shadows (non-breaking changes) Aug 22, 2024
@kaelig kaelig merged commit 11fd1c3 into main Aug 22, 2024
6 checks passed
@kaelig kaelig deleted the drwpow/shadow-proposal branch August 22, 2024 19:37
github-actions bot added a commit that referenced this pull request Aug 22, 2024
…ng changes) (#246)

SHA: 11fd1c3
Reason: push, by kaelig

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions bot added a commit that referenced this pull request Aug 22, 2024
…ng changes) (#246)

SHA: 11fd1c3
Reason: push, by kaelig

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions bot added a commit that referenced this pull request Aug 22, 2024
…ng changes) (#246)

SHA: 11fd1c3
Reason: push, by kaelig

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

2 participants