-
Notifications
You must be signed in to change notification settings - Fork 10.7k
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
Update product editor CES modal to match new designs #38592
Changes from all commits
54c2a4f
1afd336
0f73d23
fd3a3c1
476f540
9a0a27e
95d52dc
f3aaec4
6536139
c8bb1c8
cc6050e
c9ff8c7
26f6a83
63605db
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
Significance: minor | ||
Type: add | ||
|
||
Add props to allow passing a classname to the feedback modal |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
Significance: minor | ||
Type: update | ||
|
||
Update product CES modal design and fields |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,23 +1,62 @@ | ||
$modal-header-height: 84px; | ||
|
||
.woocommerce-product-mvp-feedback-modal { | ||
width: 600px; | ||
|
||
.components-modal__header { | ||
height: $modal-header-height; | ||
|
||
&-heading { | ||
font-weight: 500; | ||
font-size: 20px; | ||
} | ||
} | ||
|
||
.components-modal__content { | ||
margin-top: $modal-header-height; | ||
padding-bottom: 32px; | ||
} | ||
|
||
legend, | ||
label { | ||
color: $gray-900; | ||
} | ||
|
||
.woocommerce-product-mvp-feedback-modal__optional { | ||
color: $gray-700; | ||
} | ||
|
||
legend { | ||
font-size: 11px; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess it's a question instead of a suggestion: do we have any standards for font-sizes? 11px seems odd (literally). I see it's the same size of the design. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We've used it in a lot of places, but I'm not sure if there is a style guide somewhere for this. It would be really nice to create variables around this (e.g., |
||
font-weight: 500; | ||
line-height: 1.4; | ||
text-transform: uppercase; | ||
display: inline-block; | ||
margin-bottom: $gap-small; | ||
padding: 0; | ||
} | ||
|
||
&__subtitle { | ||
margin-top: $gap-smaller !important; | ||
} | ||
|
||
&__checkboxes { | ||
margin: $gap-small 0; | ||
display: grid; | ||
grid-template-columns: 1fr 1fr; | ||
} | ||
|
||
&__comments { | ||
margin-top: 2em; | ||
margin-bottom: 1.5em; | ||
|
||
label { | ||
display: block; | ||
font-weight: bold; | ||
text-transform: none; | ||
font-size: 14px; | ||
} | ||
|
||
textarea { | ||
width: 100%; | ||
} | ||
} | ||
|
||
&__reason, | ||
&__comments, | ||
&__email { | ||
margin-bottom: $gap-large; | ||
} | ||
} |
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 you increase the margin bottom here? It is very close to the checkboxes and the margin is bigger in the design
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.
Nice catch! Updated from
$gap-smaller
to$gap-small
.