Skip to content

Commit

Permalink
Try: Improve scrolling of navigation menu on small screens (#12644)
Browse files Browse the repository at this point in the history
* Try: Improve scrolling of navigation menu on small screens

We scroll the editing canvas, the inspector sidebar, and the block library independently at desktop breakpoints. We do this so that the sidebar inspector can stay in view even if you have scrolled far down the editing canvas, and to avoid scroll bleed.

However because the navigation sidebar (on the left) has flyout menus on the desktop breakpoints, we can't yet scroll this one separately. If a user has a bunch of plugins installed that add menu items, or a small screen, these manu items might go beyond the visible height of the viewport. To make these accessible regardless, when this happens the `body` element scrolls to let you reach them.

In this situation, there is currently an issue where the editing canvas might scroll out of view when you scroll to the bottom of the sidebar.

This PR improves that situation by making the editing canvas `position: fixed;`, same as the sidebar is. This ensures that the entire editor scrolls with you down the page, as you scroll the `body` content.

This needs a good testing. `position: fixed;` does not inherit from a relative parent, which means we have to specifiy a matrix of left margins to accommodate for the different configurations of the navigation menu: auto-collapsed, manually collapsed, or the default width. To test, please verify that everything works as intended. Please test:

- All breakpoints beyond the 600px small breakpoint.
- Fullscreen and not fullscreen modes.
- With the navigation menu auto collapsing, and explicitly collapsed.
- With the inspector sidebar present or hidden.
- With metaboxes present and not present.

* Add flex rule to fix IE issue.

* Address feedback, and fix test issue.

* Try making the test more reliable using small columns
  • Loading branch information
jasmussen authored and youknowriad committed Jan 9, 2019
1 parent dc066e9 commit 49a22e9
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 14 deletions.
59 changes: 54 additions & 5 deletions packages/edit-post/src/components/layout/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
}
}

// on mobile, toolbars behave differently
// On mobile, toolbars behave differently.
@include break-small {
padding-top: $header-height;
}
Expand All @@ -38,7 +38,7 @@
padding-top: $block-controls-height;
}

// on mobile, toolbars behave differently
// On mobile, toolbars behave differently.
@include break-small {
padding-top: $header-height + $block-toolbar-height;

Expand Down Expand Up @@ -68,11 +68,60 @@
}

.edit-post-layout__content {
position: relative;
display: flex;
min-height: 100%;
flex-direction: column;

// These rules are specific to mobile and small breakpoints.
min-height: 100%;
position: relative;

// We scroll the main editing canvas, the sidebar, and the block library separately to prevent scroll bleed.
// Because the navigation sidebar menu has "flyout" menus, we can't yet, scroll that independently, but as soon
// as we can, we should simplify these rules.
// In the mean time, if a user has a small screen and lots of plugin-added menu items in the navigation menu,
// they have to be able to scroll. To accommodate the flyout menus, we scroll the `body` element for this.
@include break-medium() {
// Because the body element scrolls the navigation sidebar, we have to use position fixed here.
// Otherwise you would scroll the editing canvas out of view when you scroll the sidebar.
position: fixed;
bottom: 0;
left: 0;
right: 0;

// Because this is scoped to break-medium and larger, the admin-bar is always this height.
top: $header-height + $admin-bar-height;

// Sadly, `position: fixed;` do not inherit boundaries from a relative parent. Due to that we have to compensate using `calc`.
min-height: calc(100% - #{ $header-height + $admin-bar-height });
height: auto; // This overrides the 100% height the element inherits from line 3.

// In this matrix, we compensate for any configurations for the presence and width of the navigation sidebar.
// This is similar to the code in the @editor-left mixin, but uses margins instead.
// Because we are beyond the medium breakpoint, we only have to worry about folded, auto-folded, and default.
margin-left: $admin-sidebar-width;

// Auto fold is when on smaller breakpoints, nav menu auto colllapses.
body.auto-fold & {
margin-left: $admin-sidebar-width-collapsed;

@include break-large() {
margin-left: $admin-sidebar-width;
}
}

// Sidebar manually collapsed.
body.folded & {
margin-left: $admin-sidebar-width-collapsed;
}

// Undo the above rules for fullscreen mode.
body.is-fullscreen-mode & {
margin-left: 0 !important;
position: relative;
top: inherit;
}
}

// Pad the scroll box so content on the bottom can be scrolled up.
padding-bottom: 50vh;
@include break-small {
Expand All @@ -95,7 +144,7 @@
}

.edit-post-visual-editor {
flex-grow: 1;
flex: 1 1 auto;

// In IE11 flex-basis: 100% cause a bug where the metaboxes area overlap with the content area.
// But it works as expected without it.
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/specs/__snapshots__/writing-flow.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ exports[`adding blocks Should navigate inner blocks with arrow keys 1`] = `
<!-- wp:columns -->
<div class=\\"wp-block-columns has-2-columns\\"><!-- wp:column -->
<div class=\\"wp-block-column\\"><!-- wp:paragraph -->
<p>First column paragraph</p>
<p>First col</p>
<!-- /wp:paragraph --></div>
<!-- /wp:column -->
<!-- wp:column -->
<div class=\\"wp-block-column\\"><!-- wp:paragraph -->
<p>Second column paragraph</p>
<p>Second col</p>
<!-- /wp:paragraph --></div>
<!-- /wp:column --></div>
<!-- /wp:columns -->
Expand Down
11 changes: 4 additions & 7 deletions test/e2e/specs/writing-flow.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,14 @@ describe( 'adding blocks', () => {
await page.keyboard.press( 'Enter' );
await page.keyboard.type( '/columns' );
await page.keyboard.press( 'Enter' );
await page.keyboard.type( 'First column paragraph' );
await page.keyboard.type( 'First col' );

// Arrow down should navigate through layouts in columns block (to
// its default appender). Two key presses are required since the first
// will land user on the Column wrapper block.
await page.keyboard.press( 'ArrowDown' );
await page.keyboard.press( 'ArrowDown' );
await page.keyboard.type( 'Second column paragraph' );
await page.keyboard.type( 'Second col' );

// Arrow down from last of layouts exits nested context to default
// appender of root level.
Expand All @@ -40,22 +40,19 @@ describe( 'adding blocks', () => {
// Arrow up into nested context focuses last text input
await page.keyboard.press( 'ArrowUp' );
activeElementText = await page.evaluate( () => document.activeElement.textContent );
expect( activeElementText ).toBe( 'Second column paragraph' );
expect( activeElementText ).toBe( 'Second col' );

// Arrow up in inner blocks should navigate through (1) column wrapper,
// (2) text fields.
// We need to arrow up key presses in the paragraph block because it shows up in two lines.
await page.keyboard.press( 'ArrowUp' );
await page.keyboard.press( 'ArrowUp' );
await page.keyboard.press( 'ArrowUp' );
activeElementText = await page.evaluate( () => document.activeElement.textContent );
expect( activeElementText ).toBe( 'First column paragraph' );
expect( activeElementText ).toBe( 'First col' );

// Arrow up from first text field in nested context focuses column and
// columns wrappers before escaping out.
let activeElementBlockType;
await page.keyboard.press( 'ArrowUp' );
await page.keyboard.press( 'ArrowUp' );
activeElementBlockType = await page.evaluate( () => (
document.activeElement.getAttribute( 'data-type' )
) );
Expand Down

0 comments on commit 49a22e9

Please sign in to comment.