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 selecting separator block #14854

Merged
merged 2 commits into from
Apr 10, 2019
Merged

Fix selecting separator block #14854

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
82e9364
fix selecting separator block
ashwin-pc Apr 6, 2019
43773f1
center align the inserter toggle
ashwin-pc Apr 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
3 changes: 2 additions & 1 deletion packages/block-editor/src/components/block-list/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -737,10 +737,11 @@
left: 0;
right: 0;
justify-content: center;
height: $block-padding + 8px;

// Show a clickable plus.
.block-editor-inserter__toggle {
Copy link
Contributor

Choose a reason for hiding this comment

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

Below this line, it says margin-top: -4px. If you change that to -6px, it better vertically centers the plus between the blocks. This was not a regression of this PR, but would be nice to fix while you're in here.

Copy link
Contributor Author

@ashwin-pc ashwin-pc Apr 8, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@jasmussen yes that sounds good. I had considered this but avoided making this change as i wasn't sure how some of the E2E tests would handle it if the inserter changed its position, especially since this is only a stop gap fix. Would you still recommend it considering that this is an additional thing to revert when the fix does come in? Also would it be a good idea to add a comment there to indicate that this is indeed a stopgap fix? So that when the real fix does come along these changes are reverted.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe. Or maybe we grow to like the change enough to keep it, even if we also fix the other issue.

I can go either way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. I've made the change to centre align it. I went with -8px instead of -6px as it accurately centre aligns it.

margin-top: -4px;
margin-top: -8px;
border-radius: 50%;
color: $blue-medium-focus;
background: $white;
Expand Down