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.

Sign up for GitHub

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

Fix checkbox alignment issues in options and block manager modals (#16860) #16863

Merged
merged 9 commits into from
Aug 9, 2019
+101 −77
Merged

Fix checkbox alignment issues in options and block manager modals (#16860) #16863

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
24bfbc9
Fix checkbox alignment issues in options and block manager modals
jffng Aug 1, 2019
de682a0
replace media query with mixin
jffng Aug 2, 2019
f3312ca
Set height of label
jffng Aug 2, 2019
f7de824
Refactor checkmark and input in single container
jffng Aug 5, 2019
6b0a212
Move variables to global stylesheets
jffng Aug 5, 2019
addf8f3
make checkbox input size variables local to component styles
jffng Aug 6, 2019
52bcc30
Merge branch 'fix/misplaced-checkmarks'
jffng Aug 8, 2019
60aaaa0
Merge branch 'master' of github.com:WordPress/gutenberg
jffng Aug 8, 2019
09d5242
rebase master and update snapshots
jffng Aug 8, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions assets/stylesheets/_variables.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,5 @@ $block-bg-padding--h: $block-side-ui-width + $block-side-ui-clearance; // paddin
// Buttons & UI Widgets
$radius-round-rectangle: 4px;
$radius-round: 50%;
$input-size: 16px;
Copy link
Contributor

Choose a reason for hiding this comment

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

To be honest, I'm not certain about these variables being global here. I think it only make sense if we use them consistently in all "inputs" otherwise it's going to create confusion. For example, why the size of the TextControl component doesn't use it?...

Thoughts @kjellr maybe we can leave them in the component for now, and maybe rename them to make it clear that it's just about the checkbox?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I just conferred with @jasmussen about this, and while we haven't usually declared variables within components in the past, we can see how it would make sense to allow them.

Feel free to move these back, @jffng (and sorry for the back and forth!). Might also want to make the names just a little more specific too, as Riad noted.

Thanks!

$input-size-sm: 25px; // width + height for small viewports
5 changes: 4 additions & 1 deletion assets/stylesheets/_z-index.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,10 @@ $z-layers: (

// Make sure corner handles are above side handles for ResizableBox component
".components-resizable-box__side-handle": 1,
".components-resizable-box__corner-handle": 2
".components-resizable-box__corner-handle": 2,

// Make sure block manager sticky category titles appear above the options
".edit-post-manage-blocks-modal__category-title": 1
);

@function z-index( $key ) {
Expand Down
24 changes: 13 additions & 11 deletions packages/components/src/checkbox-control/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,20 @@ function CheckboxControl( { label, className, heading, checked, help, instanceId

return (
<BaseControl label={ heading } id={ id } help={ help } className={ className }>
<input
id={ id }
className="components-checkbox-control__input"
type="checkbox"
value="1"
onChange={ onChangeValue }
checked={ checked }
aria-describedby={ !! help ? id + '__help' : undefined }
{ ...props }
/>
<label className="components-checkbox-control__label" htmlFor={ id }>
<span className="components-checkbox-control__input-container">
<input
id={ id }
className="components-checkbox-control__input"
type="checkbox"
value="1"
onChange={ onChangeValue }
checked={ checked }
aria-describedby={ !! help ? id + '__help' : undefined }
{ ...props }
/>
{ checked ? <Dashicon icon="yes" className="components-checkbox-control__checked" role="presentation" /> : null }
</span>
<label className="components-checkbox-control__label" htmlFor={ id }>
{ label }
</label>
</BaseControl>
Expand Down
48 changes: 25 additions & 23 deletions packages/components/src/checkbox-control/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,17 @@
cursor: pointer;
display: inline-block;
line-height: 0;
height: 16px;
margin: 0 4px 0 0;
outline: 0;
padding: 0 !important;
text-align: center;
vertical-align: middle;
width: 16px;
min-width: 16px;
vertical-align: top;
width: $input-size-sm;
height: $input-size-sm;
@include break-small() {
height: $input-size;
width: $input-size;
}
-webkit-appearance: none;
box-shadow: inset 0 1px 2px rgba(0, 0, 0, 0.1);
transition: 0.05s border-color ease-in-out;
Expand All @@ -39,34 +42,33 @@
}
}

.components-checkbox-control__label {
.components-checkbox-control__input-container {
position: relative;
display: inline-block;
margin-right: 12px;
vertical-align: middle;
line-height: 1;
width: $input-size-sm;
height: $input-size-sm;
@include break-small() {
width: $input-size;
height: $input-size;
}
}

svg.dashicon.components-checkbox-control__checked {
width: 21px;
height: 21px;
fill: #fff;
cursor: pointer;
position: absolute;
left: -31px;
bottom: -3px;
left: -4px;
top: -2px;
width: 31px;
height: 31px;
@include break-small() {
width: 21px;
height: 21px;
left: -3px;
}
user-select: none;
pointer-events: none;
}

@media screen and ( max-width: 728px ) {
.components-checkbox-control__input[type="checkbox"] {
width: 25px;
height: 25px;
}

svg.dashicon.components-checkbox-control__checked {
width: 31px;
height: 31px;
left: -41px;
bottom: -9px;
}
}
3 changes: 2 additions & 1 deletion packages/edit-post/src/components/manage-blocks-modal/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
top: 0;
padding: $panel-padding 0;
background-color: $white;
z-index: z-index(".edit-post-manage-blocks-modal__category-title");
Copy link
Contributor

Choose a reason for hiding this comment

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

How is this change related?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The headings for the block manager modal use position: sticky, creating a "sticky container" parent. Using a position: relative element (the wrapper for the input to position the checkbox) within the same sticky parent container, the browser treats them as in the same stacking context:

Screen Recording 2019-08-05 at 03 17 PM

Adding a z-index to the header makes the sticky header remain on top:

Screen Recording 2019-08-05 at 03 20 PM


.components-base-control__field {
margin-bottom: 0;
Expand Down Expand Up @@ -88,7 +89,7 @@
margin: 0;
}

.components-modal__content & input[type="checkbox"] {
.components-modal__content &.components-checkbox-control__input-container {
margin: 0 $grid-size;
}

Expand Down