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

Try: Remove canvas padding. #22213

Merged
merged 3 commits into from
Jun 22, 2020
Merged

Try: Remove canvas padding. #22213

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4370b73
Try: Remove canvas padding.
jasmussen May 8, 2020
32d0b0c
Update snapshot.
jasmussen Jun 22, 2020
6e0ff30
Try: Fix test.
jasmussen Jun 22, 2020
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
28 changes: 0 additions & 28 deletions 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 @@ -694,31 +694,3 @@
.is-dragging-components-draggable .components-tooltip {
display: none;
}


// Add side padding for the canvas, currently edit-post-visual-editor.
// The purpose of this padding is to ensure that on small viewports, there is
// room for the block border that sits 14px ($block-padding) offset from the
// block footprint.
// These paddings and margins are removed from the BlockPreview component's style
// Any change need to be reflected there.
.block-editor-block-list__layout.is-root-container {
padding-left: $block-padding;
padding-right: $block-padding;

@include break-small() {
padding-left: $block-side-ui-width;
padding-right: $block-side-ui-width;
}

// Full-wide. (to account for the paddings added above)
> .wp-block[data-align="full"] {
margin-left: -$block-padding;
margin-right: -$block-padding;

@include break-small() {
margin-left: -$block-side-ui-width;
margin-right: -$block-side-ui-width;
}
}
}
10 changes: 7 additions & 3 deletions packages/e2e-tests/specs/editor/various/__snapshots__/rtl.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ exports[`RTL should arrow navigate 1`] = `

exports[`RTL should arrow navigate between blocks 1`] = `
"<!-- wp:paragraph -->
<p>٠<br>١</p>
<p>٠</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p>٠<br>١<br>٢</p>
<p><br>١١٠<br><br>٢</p>
<!-- /wp:paragraph -->"
`;

Expand All @@ -24,7 +24,11 @@ exports[`RTL should merge backward 1`] = `

exports[`RTL should merge forward 1`] = `
"<!-- wp:paragraph -->
<p>٠١</p>
<p>٠</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p></p>
<!-- /wp:paragraph -->"
`;

Expand Down
6 changes: 6 additions & 0 deletions packages/e2e-tests/specs/editor/various/typewriter.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ describe( 'TypeWriter', () => {
// Create first block.
await page.keyboard.press( 'Enter' );

// Create second block.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ellatrix Not urgent, but when you have a moment I'd love your take on why this specific commit makes the tests pass.

For context, this PR changes the padding of the editing canvas to be smaller. This leaves more room for themes that don't add this padding themselves, i.e. 2020 which is being used for these tests.

The tests failing were

TypeWriter › should maintain caret position
TypeWriter › should maintain caret position after leaving last editable

I tested both of those using npm run test-e2e packages/e2e-tests/specs/edi tor/various/typewriter.test.js -- --puppeteer-interactive, passing on master but failing on this branch prior to this commit.

My guess is that with the padding as it was, there was enough room on the canvas for the blocks to fit without the typewriter effect setting in. This would explain why simply adding some enter-key presses fixed the test for me.

But it does suggest that this particular test is very sensitive to things style properties like padding, margin, line-height and font size. For example if TwentyTwenty updates the editor style to add this padding back, or to change a block margin, this test will likely fail again. Is there a way to make it more resilient than just adding more blocks? Or conversely, is simply adding more blocks an okay way to fix this test?

await page.keyboard.press( 'Enter' );

const initialPosition = await getCaretPosition();

// The page shouldn't be scrolled when it's being filled.
Expand Down Expand Up @@ -118,8 +121,11 @@ describe( 'TypeWriter', () => {
await page.keyboard.press( 'Enter' );
// Create second block.
await page.keyboard.press( 'Enter' );
// Create third block.
await page.keyboard.press( 'Enter' );
// Move to first block.
await page.keyboard.press( 'ArrowUp' );
await page.keyboard.press( 'ArrowUp' );

const initialPosition = await getCaretPosition();

Expand Down
11 changes: 1 addition & 10 deletions packages/edit-post/src/components/visual-editor/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,10 @@
height: 0;
}

// Ideally this wrapper div is not needed but if we waant to match the positionning of blocks
// Ideally this wrapper div is not needed but if we want to match the positioning of blocks
// .block-editor-block-list__layout and block-editor-block-list__block
// We need to have two DOM elements.
.edit-post-visual-editor__post-title-wrapper {
// This padding is needed to match the block root container padding
padding-left: $block-padding;
padding-right: $block-padding;

@include break-small() {
padding-left: $block-side-ui-width;
padding-right: $block-side-ui-width;
}

.editor-post-title {
// Center.
margin-left: auto;
Expand Down
8 changes: 8 additions & 0 deletions packages/editor/src/editor-styles.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,16 @@ body {
font-size: $editor-font-size;
line-height: $editor-line-height;
color: $dark-gray-primary;
padding: 10px;
}

// For full-wide blocks, we compensate for these 10px.
.block-editor-block-list__layout.is-root-container > .wp-block[data-align="full"] {
margin-left: -10px;
margin-right: -10px;
}


/* Headings */
// These follow a Major Third type scale
// https://type-scale.com/?size=16&scale=1.250&text=A%20Visual%20Type%20Scale&font=Noto%20Serif&fontweight=600
Expand Down