Skip to content

Try: Larger drag handles #10331

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

Merged
chrisvanpatten merged 5 commits into from
Oct 4, 2018
Merged

Try: Larger drag handles #10331

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d8acfdc
Try larger drag handles on the spacer block
chrisvanpatten Oct 4, 2018
b02d52b
Move resize styles to mixins, document use of !important, apply to im…
chrisvanpatten Oct 4, 2018
9091b5c
Use BEM-ish/more descriptive names for the mixins
chrisvanpatten Oct 4, 2018
235e8dc
Un-set inline styles on drag handles to avoid !important
chrisvanpatten Oct 4, 2018
a7060ae
Add some full stops, full stop.
chrisvanpatten Oct 4, 2018
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
1 change: 1 addition & 0 deletions edit-post/assets/stylesheets/_variables.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ $icon-button-size: 36px;
$icon-button-size-small: 24px;
$inserter-tabs-height: 36px;
$block-toolbar-height: $block-controls-height + $border-width;
$resize-handler-size: 16px;

// Blocks
$block-padding: 14px; // padding of nested blocks
Expand Down
32 changes: 23 additions & 9 deletions packages/block-library/src/spacer/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
.block-library-spacer__resize-container.is-selected {
.block-library-spacer__resize-handler-top,
.block-library-spacer__resize-handler-bottom {
display: block;
}
Expand All @@ -9,17 +8,32 @@
background: $light-gray-200;
}

.block-library-spacer__resize-handler-top,
.block-library-spacer__resize-handler-bottom {
display: none;
border-radius: 50%;
border: 2px solid $white;
width: 15px !important;
height: 15px !important;

// Make the container larger so there's a larger grabbable area
$resize-handler-container-size: $resize-handler-size + ($grid-size-small * 2);

// Offset the handle's position
position: absolute;
background: theme(primary);
left: calc(50% - #{$resize-handler-container-size / 2}) !important;
Copy link
Member

@tofumatt tofumatt Oct 4, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

!important 😱

For real though, why are these here? We shouldn't need them, and if we really do we should have comments.

(But we shouldn't need them 😄)

Copy link
Contributor

@gwwar gwwar Oct 4, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

+1 on avoiding !important when we're able to. I see that there's an inline style that includes a left rule. If possible could we try to update the related resizing JS to take this in account instead?

Adding !important rules makes this incredibly difficult to override, folks that would like other styles, basically end up needing to add another !important rule, and hope that the rule is read last.

Copy link
Contributor Author

@chrisvanpatten chrisvanpatten Oct 4, 2018

Choose a reason for hiding this comment

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

@gwwar @tofumatt Agreed, but the resize component we're using here applies inline styles which we need to override. However I also see that they support a style prop so I'll see if I can just disable the inline styles.

bottom: calc(#{$resize-handler-container-size / 2} * -1) !important;

// Apply the width/height of the container size plus padding so the handle
// sits in the middle of the grabbable area
Copy link
Member

@tofumatt tofumatt Oct 4, 2018

Choose a reason for hiding this comment

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

Nitpick but it'd be nice if all of these comments ended in fullstops. I wish we had a linting rule for that! 😓

Copy link
Contributor Author

@chrisvanpatten chrisvanpatten Oct 4, 2018

Choose a reason for hiding this comment

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

I like nitpicks 😜 I'll get it updated! Agreed, a linting rule would be nice, but maybe this could be put into contributing docs or something too?

Copy link
Member

@tofumatt tofumatt Oct 4, 2018

Choose a reason for hiding this comment

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

I would approve such a pull request! 😄

width: $resize-handler-container-size !important;
height: $resize-handler-container-size !important;
padding: $grid-size-small;
}

.block-library-spacer__resize-handler-bottom::before {
display: block;
content: "";
border: 2px solid $white;
border-radius: 50%;
width: $resize-handler-size !important;
height: $resize-handler-size !important;
padding: 0 3px 3px 0;
background: theme(primary);
cursor: se-resize;
left: 50% !important;
margin-left: -7.5px;
}