This repository has been archived by the owner on Feb 23, 2024. It is now read-only.
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.
Product SKU: Adds support for color, typography, and spacing #8913
Product SKU: Adds support for color, typography, and spacing #8913
Changes from 5 commits
ef08d62
157cba8
13e27b0
8db4da5
a007701
35f82f6
705599d
9e10039
8c4e4c5
4b53363
d0384a5
5432e0e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Heads-up that this line introduced an ESLint warning.
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.
ESLint warning is present wherever we use __experimentalGetSpacingClassesAndStyles, therefore I thought it was acceptable. However, I will add the following lines to ignore the warning going forward 🙂
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.
If we can remove
__experimentalSelector
, then we could make these styles available in WC core as well, so only the experimental ones would be gated to the feature plugin. 🙂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.
That makes sense. I will make the necessary changes 🤝
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.
Is
__experimentalSelector
required? I tried removing it and everything seemed to keep working as expected. 🤔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.
Good catch! I think we can remove
__experimentalSelector
flag.