Skip to content

feat(dropshadows): adds dropshadows to commons #2419

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

Closed
wants to merge 4 commits into from

Conversation

jenndiaz
Copy link
Contributor

@jenndiaz jenndiaz commented Jan 11, 2024

Description

Adds S2 Drop Shadow styling to commons
Allowing for dropshadows to be imported @import "../commons/dropshadows.css"; and used with in classes @inherit: %spectrum-DropShadow-key;

TO DO:

  • update with S2 tokens once they are released

How and where has this been tested?

Please tag yourself on the tests you've marked complete to confirm the tests have been run by someone other than the author.

Validation steps

Regression testing

Validate:

  1. The documentation pages for at least two other components are still loading, including:
  • The pages render correctly, are accessible, and are responsive.
  1. If components have been modified, VRTs have been run on this branch:
  • VRTs have been run and looked at.
  • Any VRT changes have been accepted (by reviewer and/or PR author), or there are no changes.

Screenshots

To-do list

  • I have read the contribution guidelines.
  • I have updated relevant storybook stories and templates.
  • I have tested these changes in Windows High Contrast mode.
  • If my change impacts other components, I have tested to make sure they don't break.
  • If my change impacts documentation, I have updated the documentation accordingly.
  • ✨ This pull request is ready to merge. ✨

pfulton and others added 4 commits January 8, 2024 16:09
BREAKING CHANGE: Merges Express and Spectrum to create Spectrum 2
Includes new color values for Spectrum 2
@pfulton pfulton force-pushed the spectrum-two branch 4 times, most recently from 31afc05 to 909f232 Compare January 22, 2024 20:15
@pfulton pfulton force-pushed the spectrum-two branch 3 times, most recently from 9a01c5a to 1b959b3 Compare February 5, 2024 21:12
*/

%spectrum-DropShadow-key {
--spectrum-drop-shadow-color-key: var(--spectrum-drop-shadow-color);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it be valuable to split out the variable definitions from the css property definitions? Just trying to think of examples where someone might want to variables and properties to source from different levels (properties at root for example).

Comment on lines +20 to +21
"concat-cli": "^4.0.0",
"npm-run-all": "^4.1.5",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think these might be added from an old version. We don't use npm-run-all or concat-cli anymore for token building.

@castastrophe
Copy link
Collaborator

Just a note for future to use @import "@spectrum-css/commons/dropshadows.css" instead and document that the commons package needs to be added as a devDependency.

@pfulton pfulton force-pushed the spectrum-two branch 2 times, most recently from 98335ab to fec70b3 Compare February 26, 2024 19:47
@pfulton pfulton force-pushed the spectrum-two branch 2 times, most recently from 7c86724 to 0a218bc Compare March 28, 2024 14:22
@castastrophe castastrophe force-pushed the spectrum-two branch 5 times, most recently from 0f20c5b to 9ff2a2c Compare April 5, 2024 14:36
@pfulton pfulton force-pushed the spectrum-two branch 2 times, most recently from e03868f to b3b3aa5 Compare April 15, 2024 19:32
@pfulton pfulton force-pushed the spectrum-two branch 2 times, most recently from f3dd9ff to 9931a8e Compare April 19, 2024 18:22
@castastrophe castastrophe force-pushed the spectrum-two branch 5 times, most recently from 6dbc850 to ae7aedd Compare April 26, 2024 20:37
@pfulton pfulton force-pushed the spectrum-two branch 2 times, most recently from e0e0fd2 to 698e904 Compare May 3, 2024 18:39
@jawinn
Copy link
Collaborator

jawinn commented Jun 3, 2024

This was great exploratory work, thanks Jenn!
The followup PR that finalizes the drop shadow foundations info is #2674 (merging soon into spectrum-two).

@jawinn jawinn closed this Jun 3, 2024
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.

4 participants