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.
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
MWPW-158473: CCD Suggested Card [Merch card] #2927
MWPW-158473: CCD Suggested Card [Merch card] #2927
Changes from 6 commits
29a16b0
cd361b9
d47b6ff
1894a7a
eb40fb4
329be3a
0b1aef8
e20123a
f545c53
254ef7a
937713f
59285a0
1dd8ea9
5e5924d
eb156cc
5302051
bdf845d
d2b50c4
bdd6322
d8eb9e9
d1ba823
fed743d
48d5fc4
a7ac03d
0aa1042
521ac4e
b2e797e
d07b721
cd61ece
54dd79d
f5c38d3
aff1f4a
046f140
b5ef73c
8069853
2f47a9d
181a47d
0f39d38
a77246f
53faa8d
1bd7cd7
bf98d49
d697854
5f6c4dd
7d3b828
fb50fdc
dd9b869
f179e09
f8fcef8
655afb4
13fe806
755e6e6
fb07228
c6d7712
2c73de1
eae4854
2fd5647
5b8bdf6
2ddea92
619f690
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.
Also, let's not use
--consonant
prefix for CCD cards are they are not meant to be one.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.
Removed consonant prefix
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.
can we use corresponding spectrum variable and fallback to
#6D6D6D;
?Thus, it should work with dark theme automatically.
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.
using the spectrum var inside merch var and the hex value as fallback didn't work. But I changed the reference to use spectrum var and hex as fallback directly in the selectors that use it.
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.
I suggest to adopt
--merch-card-ccd-*
for ccd cards.@npeltier thoughts?
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.
i was thinking that we do need some rules indeed :)
we'll soon have to introduce concept of surface anyway, and i was wondering if with that in mind we still do need to put CCD in our names & variables
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 should be part of the card CSS, rather than the document's
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.
There is a problem with doing what you suggest: Global styles override VariantLayout styles. I suggest we discuss how to proceed with this in our tech call but it wouldn't hurt to leave it as is in the meantime.
Check warning on line 77 in libs/features/mas/web-components/src/variants/variant-layout.js
Codecov / codecov/patch
libs/features/mas/web-components/src/variants/variant-layout.js#L77