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

Support arrays of shadows, inset property #114

Merged
merged 1 commit into from
Sep 9, 2023
Merged

Support arrays of shadows, inset property #114

merged 1 commit into from
Sep 9, 2023

Conversation

drwpow
Copy link
Collaborator

@drwpow drwpow commented Sep 9, 2023

Changes

Fixes #88. Allows arrays of shadows in a backwards-compatible way in anticipation of growing community support. (older versions of Cobalt should still work with plugins. And declaring shadows as an object works, of course, because that’s still technically the spec.

Also adds the addition of the inset keyword which also seemed to win over community support.

How to Review

  • Tests updated; tests should pass

@changeset-bot
Copy link

changeset-bot bot commented Sep 9, 2023

🦋 Changeset detected

Latest commit: c1d7e03

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@cobalt-ui/plugin-sass Major
@cobalt-ui/plugin-css Minor
@cobalt-ui/core Minor
@cobalt-ui/plugin-js Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

}
case 'shadow': {
return transformShadow(value as typeof token.$value);
let {value, originalVal} = getMode(token, options?.mode);
if (typeof originalVal === 'string') {
Copy link
Collaborator Author

@drwpow drwpow Sep 9, 2023

Choose a reason for hiding this comment

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

So a big, annoying change in plugin-css was:

Before: all tokens would be resolved to CSS variables BEFORE transformation

After: there is no resolution step; every transformer handles looking up aliases and forming the token as-needed

The reason for this change is there’s a lot of nuance and complexity between token values and aliases. Specifically with composite tokens—because composite tokens can either be fully-aliased or partially aliased the best way to deal with that complexity is by first splitting apart tokens by type.

As a result of this PR, the code feels a bit messier by having alias resolution appear once per type. But overall is much more stable than it was before. As evidenced by removing hacks like CSS_VAR_RE.test(value) where we didn’t know if something had been transformed or not yet (which we should know; and now we do)

Copy link
Collaborator Author

@drwpow drwpow Sep 9, 2023

Choose a reason for hiding this comment

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

Also, as a result of this change, there are no more transform methods for composite types (e.g. transformShadow()). The reason is, again, because composite types can either be fully- or partially-aliased, the only way to do that properly when factoring in modes is within the full context of the defaultTransformer().

So now, the defaultTransformer() just keeps all context, and handles transforming composite token types itself (though we can still keep logic clean by separating out base-level token transformers since those get reused).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Anyways, TL;DR, this also came up as a blocker for #113 as well, but this is the solution (I think). Anyway, we’ll know once I merge this and take another crack at that.


@supports (color: color(display-p3 1 1 1)) {
:root {
--ds-shadow-inset: inset 0 4px 8px 0 color(display-p3 0 0 0 / 0.14901960784313725);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

🎉

:root {
--ds-shadow-base: var(--ds-shadow-simple);
--ds-shadow-inset: inset 0 4px 8px 0 #00000026;
--ds-shadow-layered: 0 1px 1px 0 #0000001f, 0 2px 2px 0 #0000001f, 0 4px 4px 0 #0000001f, 0 8px 8px 0 #0000001f, 0 16px 16px 0 #0000001f;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

🎉 🎉 🎉

@drwpow drwpow merged commit 9f62035 into main Sep 9, 2023
8 checks passed
@drwpow drwpow deleted the shadow-arrays branch September 9, 2023 22:46
@github-actions github-actions bot mentioned this pull request Sep 9, 2023
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.

Allow arrows of shadows
1 participant