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.

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

Resizable editor experiment #19082

Merged
merged 39 commits into from
Feb 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
dfaa1b7
Resizable editor experiment
tellthemachines Dec 12, 2019
2720b04
Add Jorge's simulate media query component
tellthemachines Dec 18, 2019
6f0ca43
Integrate Jorge's solution with resizable editor
tellthemachines Dec 18, 2019
600a9ff
docs update
tellthemachines Dec 18, 2019
122ccee
Add stylesheet list as editor setting.
tellthemachines Jan 13, 2020
75f7d28
Stage management for editing canvas device type.
tellthemachines Jan 14, 2020
812e31a
Fix IE incompatibility.
tellthemachines Jan 15, 2020
39370bc
Dropdown styling.
tellthemachines Jan 15, 2020
a9f5d54
Fixed styling bugs
tellthemachines Jan 16, 2020
21f4a16
Classic preview button for mobile.
tellthemachines Jan 16, 2020
63bbfa5
Accurate styles on window resize.
tellthemachines Jan 16, 2020
aaec420
Fix preview button unit tests.
tellthemachines Jan 16, 2020
ad4c669
Docs updates.
tellthemachines Jan 17, 2020
86484be
Min heights and width adjustment.
tellthemachines Jan 17, 2020
6dd6e75
Address styling issues.
tellthemachines Jan 20, 2020
3f70996
Attempt to fix block inserter menu.
tellthemachines Jan 20, 2020
e8e1d92
Address review feedback.
tellthemachines Jan 21, 2020
f56765e
Remove unused styles.
tellthemachines Jan 21, 2020
93e6e45
Fix e2e tests.
tellthemachines Jan 22, 2020
1e446e0
Remove non-existent stylesheet
tellthemachines Jan 22, 2020
860670d
Add support for parsing concatenated stylesheets
tellthemachines Jan 30, 2020
4ecef11
Fix lint error.
tellthemachines Jan 30, 2020
d23bac2
Address styling feedback
tellthemachines Jan 30, 2020
9c42b28
Apply style change on focus.
tellthemachines Jan 30, 2020
3544005
Styling tweaks.
tellthemachines Jan 30, 2020
a6668be
Parse all stylesheets from same domain
tellthemachines Feb 5, 2020
51e578b
lint
tellthemachines Feb 5, 2020
c28e1e6
Lint.
tellthemachines Feb 6, 2020
231a89f
Move canvas resizing to visual editor
tellthemachines Feb 9, 2020
718f246
Lint and style fixes
tellthemachines Feb 9, 2020
f832790
Merge branch 'master' into try/resizable-editor
tellthemachines Feb 10, 2020
7d2bf12
Make preview dropdown edit-post component.
tellthemachines Feb 10, 2020
50370fc
Styling adjustments.
tellthemachines Feb 10, 2020
61471d0
Add inline docs and pass marker as parameter.
tellthemachines Feb 10, 2020
0696f6a
Rename selector.
tellthemachines Feb 10, 2020
dd53ad9
Miscellaneous styling fixes
tellthemachines Feb 10, 2020
aedf6bc
Early return
tellthemachines Feb 10, 2020
3356171
Button height adjustment.
tellthemachines Feb 11, 2020
ba531c4
Update docs.
tellthemachines Feb 11, 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
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,18 @@ _Returns_

- `?string`: Adjacent block's client ID, or null if none exists.

<a name="getPreviewDeviceType" href="#getPreviewDeviceType">#</a> **getPreviewDeviceType**

Returns the current editing canvas device type.

_Parameters_

- _state_ `Object`: Global application state.

_Returns_

- `string`: Device type.

<a name="getPreviousBlockClientId" href="#getPreviousBlockClientId">#</a> **getPreviousBlockClientId**

Returns the previous block's client ID from the given reference start ID.
Expand Down Expand Up @@ -1214,6 +1226,18 @@ _Parameters_

- _isNavigationMode_ `string`: Enable/Disable navigation mode.

<a name="setPreviewDeviceType" href="#setPreviewDeviceType">#</a> **setPreviewDeviceType**

Returns an action object used to toggle the width of the editing canvas

_Parameters_

- _deviceType_ `string`:

_Returns_

- `Object`: Action object.

<a name="setTemplateValidity" href="#setTemplateValidity">#</a> **setTemplateValidity**

Returns an action object resetting the template validity.
Expand Down
6 changes: 6 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions packages/block-editor/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,15 @@ _Related_

Undocumented declaration.

<a name="useSimulatedMediaQuery" href="#useSimulatedMediaQuery">#</a> **useSimulatedMediaQuery**

Function that manipulates media queries from stylesheets to simulate a given viewport width.

_Parameters_

- _marker_ `string`: CSS selector string defining start and end of manipulable styles.
- _width_ `number`: Viewport width to simulate.

<a name="Warning" href="#Warning">#</a> **Warning**

Undocumented declaration.
Expand Down
1 change: 1 addition & 0 deletions packages/block-editor/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
"@wordpress/viewport": "file:../viewport",
"@wordpress/wordcount": "file:../wordcount",
"classnames": "^2.2.5",
"css-mediaquery": "^0.1.2",
"diff": "^3.5.0",
"dom-scroll-into-view": "^1.2.1",
"inherits": "^2.0.3",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ html {

.block-editor-editor-skeleton__content {
flex-grow: 1;
background-color: $light-gray-700;

// Treat as flex container to allow children to grow to occupy full
// available height of the content area.
Expand All @@ -68,9 +69,9 @@ html {
// On Mobile the header is fixed to keep HTML as scrollable.
// Beyond the medium breakpoint, we allow the sidebar.
// The sidebar should scroll independently, so enable scroll here also.
@include break-medium() {
overflow: auto;
}

overflow: auto;

}

.block-editor-editor-skeleton__sidebar {
Expand Down
1 change: 1 addition & 0 deletions packages/block-editor/src/components/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,3 +92,4 @@ export { default as WritingFlow } from './writing-flow';
*/

export { default as BlockEditorProvider } from './provider';
export { default as useSimulatedMediaQuery } from './simulate-media-query';
3 changes: 2 additions & 1 deletion packages/block-editor/src/components/inserter/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,11 @@ $block-inserter-search-height: 38px;
height: 100%;
display: flex;
width: auto;
min-width: 400px;
Copy link
Member

Choose a reason for hiding this comment

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

What about viewports which are narrower than 400px?

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops, that needs to be in a media query.

position: relative;

@include break-medium {
width: 400px;
position: relative;

&.has-help-panel {
width: 700px;
Expand Down
128 changes: 128 additions & 0 deletions packages/block-editor/src/components/simulate-media-query/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
/**
Copy link
Member

@aduth aduth Feb 14, 2020

Choose a reason for hiding this comment

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

I'd expect if the default export of this file is useSimulatedMediaQuery, the folder ought to be named accordingly, i.e. use-simulated-media-query. Both for consistency, but also because it helps discoverability of the hook source when working backward from the known export.

Prior art:

* External dependencies
*/
import { filter, get } from 'lodash';
import { match } from 'css-mediaquery';

/**
* WordPress dependencies
*/
import { useEffect } from '@wordpress/element';

const ENABLED_MEDIA_QUERY = '(min-width:0px)';
const DISABLED_MEDIA_QUERY = '(min-width:999999px)';
Copy link
Member

Choose a reason for hiding this comment

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

Would someone with a setup like this mess us up? 🤭

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Uh, I guess we could add a few more digits. But I'd rather leave it as is just because I'd love to see the screenshot for that bug report 😂

Copy link
Member

Choose a reason for hiding this comment

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

I concur.


const VALID_MEDIA_QUERY_REGEX = /\((min|max)-width:[^\(]*?\)/g;

function getStyleSheetsThatMatchHostname() {
Copy link
Member

Choose a reason for hiding this comment

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

Any reasons to return early from this function if window is undefined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. This code shouldn't ever run on the server side, but better safe than sorry I guess 😅

if ( ! window ) {
return;
}
return filter(
get( window, [ 'document', 'styleSheets' ], [] ),
( styleSheet ) => {
return (
styleSheet.href &&
styleSheet.href.includes( window.location.hostname )
Copy link
Contributor

@mikeselander mikeselander Feb 18, 2020

Choose a reason for hiding this comment

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

There's a couple of issues with this simplistic of logic:

  1. I believe this excludes any styles passed in via add_editor_style as they're added in an inline style element.
  2. We should additionally be excluding any print stylesheets as they're not useful to this logic.
  3. We should also be excluding most plugin stylesheets from this list as they don't have any block-list related styles. This probably seems like a micro-optimization, but adding just ACF & Yoast can add 20-30 stylesheets to the editor view which is a lot more rules to be parsing. This would be really difficult to do on a granular basis, but I believe my next point would be able to help with this.
  4. To reduce the number of rules to parse when triggering a change, we could check for valid rules in a stylesheet and only push through those stylesheets which have parsable rules. On the site I'm integrating this into this change + culling out plugins stylesheets cut the stylesheets being passed here from 60+ to 15. This dramatically sped up the rule parsing when triggering the viewport change.

);
}
);
}

function isReplaceableMediaRule( rule ) {
if ( ! rule.media ) {
return false;
}
// Need to use "media.mediaText" instead of "conditionText" for IE support.
return !! rule.media.mediaText.match( VALID_MEDIA_QUERY_REGEX );
}

function replaceRule( styleSheet, newRuleText, index ) {
styleSheet.deleteRule( index );
styleSheet.insertRule( newRuleText, index );
}

function replaceMediaQueryWithWidthEvaluation( ruleText, widthValue ) {
return ruleText.replace( VALID_MEDIA_QUERY_REGEX, ( matchedSubstring ) => {
if (
match( matchedSubstring, {
type: 'screen',
width: widthValue,
} )
) {
return ENABLED_MEDIA_QUERY;
}
return DISABLED_MEDIA_QUERY;
} );
}
Copy link
Member

Choose a reason for hiding this comment

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

This is really cool! 😻

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Props to @jorgefilipecosta for most of the replacing logic 😁


/**
* Function that manipulates media queries from stylesheets to simulate a given viewport width.
*
* @param {string} marker CSS selector string defining start and end of manipulable styles.
* @param {number} width Viewport width to simulate.
*/
export default function useSimulatedMediaQuery( marker, width ) {
useEffect( () => {
const styleSheets = getStyleSheetsThatMatchHostname();
Copy link
Contributor

@mikeselander mikeselander Feb 18, 2020

Choose a reason for hiding this comment

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

As far as I can tell, we're pulling stylesheets anew every single time this runs which is unnecessary. I think this could be wrapped in useMemo to prevent it from constantly re-loading.

const originalStyles = [];
styleSheets.forEach( ( styleSheet, styleSheetIndex ) => {
Comment on lines +67 to +69
Copy link
Member

Choose a reason for hiding this comment

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

getStyleSheetsThatMatchHostname can return undefined:

...at which point, forEach will throw an error.

(Aside: TypeScript checking as in #18838 could have caught this)

let relevantSection = false;
for (
let ruleIndex = 0;
ruleIndex < styleSheet.cssRules.length;
++ruleIndex
) {
const rule = styleSheet.cssRules[ ruleIndex ];
if (
! relevantSection &&
!! rule.cssText.match( new RegExp( `#start-${ marker }` ) )
) {
relevantSection = true;
}

if (
relevantSection &&
!! rule.cssText.match( new RegExp( `#end-${ marker }` ) )
Copy link
Contributor

Choose a reason for hiding this comment

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

This logic is moderately useful for Gutenberg-written stylesheets but untenable for theme and plugin stylesheets. As an example, I'm currently working on porting a client over to Gutenberg and they have 200+ individual SCSS files that are used in the theme. Adding this rule that makes no sense in any other context to a stylesheet is polluting and doesn't make sense for the other 10 teams working on the site.

I would propose that a better rule would be to look for any rules with .editor-styles-wrapper or .wp-block classes in the cssText. This would allow us to not pollute stylesheets with invalid and nonsense rules.

) {
relevantSection = false;
}

if ( ! relevantSection || ! isReplaceableMediaRule( rule ) ) {
continue;
}
const ruleText = rule.cssText;
if ( ! originalStyles[ styleSheetIndex ] ) {
originalStyles[ styleSheetIndex ] = [];
}
originalStyles[ styleSheetIndex ][ ruleIndex ] = ruleText;
replaceRule(
styleSheet,
replaceMediaQueryWithWidthEvaluation( ruleText, width ),
ruleIndex
);
}
} );
return () => {
originalStyles.forEach( ( rulesCollection, styleSheetIndex ) => {
if ( ! rulesCollection ) {
return;
}
for (
let ruleIndex = 0;
ruleIndex < rulesCollection.length;
++ruleIndex
) {
const originalRuleText = rulesCollection[ ruleIndex ];
if ( originalRuleText ) {
replaceRule(
styleSheets[ styleSheetIndex ],
originalRuleText,
ruleIndex
);
}
}
} );
};
}, [ width ] );
}
14 changes: 14 additions & 0 deletions packages/block-editor/src/store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -799,6 +799,20 @@ export function updateSettings( settings ) {
};
}

/**
* Returns an action object used to toggle the width of the editing canvas
*
* @param {string} deviceType
*
* @return {Object} Action object.
*/
export function setPreviewDeviceType( deviceType ) {
return {
type: 'SET_PREVIEW_DEVICE_TYPE',
deviceType,
};
}

/**
* Returns an action object used in signalling that a temporary reusable blocks have been saved
* in order to switch its temporary id with the real id.
Expand Down
19 changes: 18 additions & 1 deletion packages/block-editor/src/store/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import {
*/
import { combineReducers } from '@wordpress/data';
import { isReusableBlock } from '@wordpress/blocks';

/**
* Internal dependencies
*/
Expand Down Expand Up @@ -1449,6 +1448,23 @@ export function automaticChangeStatus( state, action ) {
// Reset the state by default (for any action not handled).
}

/**
* Reducer returning the editing canvas device type.
*
* @param {Object} state Current state.
* @param {Object} action Dispatched action.
*
* @return {Object} Updated state.
*/
export function deviceType( state = 'Desktop', action ) {
switch ( action.type ) {
case 'SET_PREVIEW_DEVICE_TYPE':
return action.deviceType;
}

return state;
}

export default combineReducers( {
blocks,
isTyping,
Expand All @@ -1468,4 +1484,5 @@ export default combineReducers( {
lastBlockAttributesChange,
isNavigationMode,
automaticChangeStatus,
deviceType,
} );
11 changes: 11 additions & 0 deletions packages/block-editor/src/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1621,3 +1621,14 @@ export function isNavigationMode( state ) {
export function didAutomaticChange( state ) {
return !! state.automaticChangeStatus;
}

/**
* Returns the current editing canvas device type.
*
* @param {Object} state Global application state.
*
* @return {string} Device type.
*/
export function getPreviewDeviceType( state ) {
return state.deviceType;
}
19 changes: 17 additions & 2 deletions packages/block-editor/src/style.scss
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
// This tag marks the start of the styles that apply to editing canvas contents and need to be manipulated when we resize the editor.
#start-resizable-editor-section {
Copy link
Contributor

Choose a reason for hiding this comment

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

@jorgefilipecosta Can we use the simulated queries introduced here in the customizer sidebar?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I think it would work as expected in the customizer view we would useSimulatedMediaQuery with a value that forces a mobile-like view. It will probably not work magically right away and we will probably need some adjustments to make things look well on the customizer like I did in #17960.

display: none;
}

// Only add styles for components that are used inside the editing canvas here:

@import "./components/block-icon/style.scss";
@import "./components/block-inspector/style.scss";
@import "./components/block-list/style.scss";
Expand All @@ -19,11 +26,9 @@
@import "./components/colors-gradients/style.scss";
@import "./components/contrast-checker/style.scss";
@import "./components/default-block-appender/style.scss";
@import "./components/editor-skeleton/style.scss";
@import "./components/link-control/style.scss";
@import "./components/image-size-control/style.scss";
@import "./components/inner-blocks/style.scss";
@import "./components/inserter/style.scss";
@import "./components/inserter-list-item/style.scss";
@import "./components/media-replace-flow/style.scss";
@import "./components/media-placeholder/style.scss";
Expand All @@ -39,3 +44,13 @@
@import "./components/warning/style.scss";
@import "./components/writing-flow/style.scss";
@import "./hooks/anchor.scss";

// This tag marks the end of the styles that apply to editing canvas contents and need to be manipulated when we resize the editor.
#end-resizable-editor-section {
display: none;
}

// Styles for components that are used outside of the editing canvas go here:

@import "./components/editor-skeleton/style.scss";
@import "./components/inserter/style.scss";
10 changes: 10 additions & 0 deletions packages/block-library/src/editor.scss
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
// This tag marks the start of the styles that apply to editing canvas contents and need to be manipulated when we resize the editor.
#start-resizable-editor-section {
Copy link
Member

Choose a reason for hiding this comment

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

Do we need comments on these?

display: none;
}

@import "./archives/editor.scss";
@import "./audio/editor.scss";
@import "./block/editor.scss";
Expand Down Expand Up @@ -57,3 +62,8 @@
margin-top: $default-block-margin;
margin-bottom: $default-block-margin;
}

// This tag marks the end of the styles that apply to editing canvas contents and need to be manipulated when we resize the editor.
#end-resizable-editor-section {
display: none;
}
10 changes: 10 additions & 0 deletions packages/block-library/src/style.scss
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
// This tag marks the start of the styles that apply to editing canvas contents and need to be manipulated when we resize the editor.
#start-resizable-editor-section {
display: none;
}

@import "./audio/style.scss";
@import "./button/style.scss";
@import "./buttons/style.scss";
Expand Down Expand Up @@ -247,3 +252,8 @@
/*rtl:ignore*/
text-align: right;
}

// This tag marks the end of the styles that apply to editing canvas contents and need to be manipulated when we resize the editor.
#end-resizable-editor-section {
display: none;
}
Loading