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

Jump to bottom

Add simulate media queries #17946

Closed
wants to merge 10 commits into from
Closed

Add simulate media queries #17946

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
4d69626
Add simulate media queries component.
jorgefilipecosta Aug 12, 2019
026582b
Add simulate media query test code
jorgefilipecosta Oct 15, 2019
760d085
Update packages/edit-post/src/components/sidebar/settings-sidebar/ind…
jorgefilipecosta Nov 12, 2019
e24a221
Update packages/edit-post/src/components/sidebar/settings-sidebar/ind…
jorgefilipecosta Nov 12, 2019
627d1e8
Update packages/edit-post/src/components/sidebar/settings-sidebar/ind…
jorgefilipecosta Nov 12, 2019
02feda9
Update packages/edit-post/src/components/sidebar/settings-sidebar/ind…
jorgefilipecosta Nov 12, 2019
b5042e1
Update packages/edit-post/src/components/sidebar/settings-sidebar/ind…
jorgefilipecosta Nov 12, 2019
8945fc5
Update packages/edit-post/src/components/sidebar/settings-sidebar/ind…
jorgefilipecosta Nov 12, 2019
a4640f0
Update packages/edit-post/src/components/sidebar/settings-sidebar/ind…
jorgefilipecosta Nov 12, 2019
de986c8
Comments were removed
jorgefilipecosta Dec 11, 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
1 change: 1 addition & 0 deletions packages/block-editor/src/components/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,3 +87,4 @@ export { default as WritingFlow } from './writing-flow';
*/

export { default as BlockEditorProvider } from './provider';
export { default as __experimentalSimulateMediaQuery } from './simulate-media-query';
103 changes: 103 additions & 0 deletions packages/block-editor/src/components/simulate-media-query/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/**
* External dependencies
*/
import {
filter,
get,
some,
} from 'lodash';

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

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

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

function getStyleSheetsThatMatchPaths( partialPaths ) {
return filter(
get( window, [ 'document', 'styleSheets' ], [] ),
( styleSheet ) => {
return (
styleSheet.href &&
some(
partialPaths,
( partialPath ) => {
return styleSheet.href.includes( partialPath );
}
)
);
}
);
}

function isReplaceableMediaRule( rule ) {
if ( ! rule.media ) {
jorgefilipecosta marked this conversation as resolved.
Show resolved Hide resolved
return false;
}
return !! rule.conditionText.match( VALID_MEDIA_QUERY_REGEX );
}

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

function replaceMediaQueryWithWidthEvaluation( ruleText, widthValue ) {
return ruleText.replace( VALID_MEDIA_QUERY_REGEX, ( match, minOrMax, value ) => {
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice if we could actually evaluate the media query, rather than trying to extract a pixel value. The currnet implementation would miss a bunch of alternate units or complex queries.

I explored this a bit at #13203 (comment), with some approaches using iframe or an npm package.

Copy link
Member Author

Choose a reason for hiding this comment

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

The current implementation is very naive. But we can easily expand the replace mechanism to support complex queries e.g: screen and (min-width: 40em) is replaced with screen and (min-width: 999999px), we would need some logic to convert units which is not hard.
Evaluating or using a package is interesting if we want to support more complex scenarios like (hover: none) to simulate touch. The npm package would be very interesting if it relied on browser evaluation for properties not explicitly passed.
e.g:

 cssMediaquery.match('screen and (min-width: 40px)', {
    width: '1024px'
});

Returns false because we did not pass "type : 'screen'", while in a replace approach it may be true because the current device is a screen. I guess given that defaults are not assumed it will not be easy to provide a complete set of default values.

Copy link
Contributor

Choose a reason for hiding this comment

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

The npm package would be very interesting if it relied on browser evaluation for properties not explicitly passed.

Are we interested in any type other than 'screen'?. If we pass the match function type: 'screen' it'll match all media queries that don't explicitly have a different type set, so

 cssMediaquery.match('(min-width: 40px)', {
    width: '1024px',
    type: 'screen'
});

will return true.
Not quite sure how it handles things like prefers-reduced-motion though.

const integerValue = parseInt( value );
if (
( minOrMax === 'min' && widthValue >= integerValue ) ||
( minOrMax === 'max' && widthValue <= integerValue )
) {
return ENABLED_MEDIA_QUERY;
}
return DISABLED_MEDIA_QUERY;
} );
}

export default function SimulateMediaQuery( { partialPaths, width } ) {
useEffect(
() => {
const styleSheets = getStyleSheetsThatMatchPaths( partialPaths );
const originalStyles = [];
styleSheets.forEach( ( styleSheet, styleSheetIndex ) => {
for ( let ruleIndex = 0; ruleIndex < styleSheet.cssRules.length; ++ruleIndex ) {
const rule = styleSheet.cssRules[ ruleIndex ];
if ( ! 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 ) => {
Copy link
Member

Choose a reason for hiding this comment

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

I had trouble thinking in my experiment how to reliably restore styles after the simulation is complete. I think this combination of useEffect plus a consistent reference to both the original styleSheets and originalStyles should work pretty well.

I suppose there might be the off chance that if a specific rule in a stylesheet were dynamically modified elsewhere, the ruleIndex might become inaccurate at the time we restore styles. That seems like an edge case though.

if ( ! rulesCollection ) {
return;
}
for ( let ruleIndex = 0; ruleIndex < rulesCollection.length; ++ruleIndex ) {
const originalRuleText = rulesCollection[ ruleIndex ];
if ( originalRuleText ) {
jorgefilipecosta marked this conversation as resolved.
Show resolved Hide resolved
replaceRule( styleSheets[ styleSheetIndex ], originalRuleText, ruleIndex );
}
}
} );
};
},
[ partialPaths, width ]
);
return null;
}

44 changes: 42 additions & 2 deletions packages/edit-post/src/components/sidebar/settings-sidebar/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
/**
* WordPress dependencies
*/
import { Panel } from '@wordpress/components';
import { Panel, ToggleControl, RangeControl } from '@wordpress/components';
import { compose, ifCondition } from '@wordpress/compose';
import { withSelect } from '@wordpress/data';
import { BlockInspector } from '@wordpress/block-editor';
import { BlockInspector, __experimentalSimulateMediaQuery } from '@wordpress/block-editor';
import { __ } from '@wordpress/i18n';
import { useState } from '@wordpress/element';

/**
* Internal dependencies
Expand All @@ -22,6 +24,43 @@ import PageAttributes from '../page-attributes';
import MetaBoxes from '../../meta-boxes';
import PluginDocumentSettingPanel from '../plugin-document-setting-panel';

const PARTIAL_PATHS = [
Copy link
Member

Choose a reason for hiding this comment

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

How do you imagine we account for styles associated to using editor_style or style in register_block_type ?

If we can have the absolute paths for those stylesheets, I assume it wouldn't be much more of a stretch to try to also have the full paths for the editor, block library, and theme styles. The idea of hard-coding a few partial paths doesn't seem like it would be a very effective solution.

Copy link
Member Author

@jorgefilipecosta jorgefilipecosta Nov 25, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

That's one of the hardest challenges of this approach -- deciding what stylesheets should be converted.

The idea of hard-coding a few partial paths doesn't seem like it would be a very effective solution.

I agree. I used hard codding of these styles as a simpler way to test the approach. My idea is for the server to pass a setting named "media_query_rewrite_paths" containing the paths of style sheets that should be rewritten. I guess some of these styles will be common to the styles we use in for the editor styles wrapping rewrite.

Copy link
Contributor

Choose a reason for hiding this comment

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

Currently these only exist as individual stylesheets in the plugin though. In core they're all bundled together with styles that we don't want to change at all such as wp-admin, editor and component ones. So the solution will need to include making sure all our post-content-relevant styles (or all our full-site-editing-relevant styles?) are bundled as a separate stylesheet in core, and then we'll probably also want to include the theme stylesheet.

'block-library/style.css',
'block-library/theme.css',
'block-library/editor.css',
];
function TestSimulateMediaQuery() {
const [ simulationWidth, setSimulationWidth ] = useState( 400 );
const [ isSimulationEnabled, setIsSimulationEnabled ] = useState( true );
const toggleUI = (
<ToggleControl
label={ __( 'Enabled width simulation' ) }
checked={ isSimulationEnabled }
onChange={ ( newValue ) => setIsSimulationEnabled( newValue ) }
/>
);
if ( ! isSimulationEnabled ) {
return toggleUI;
}
return (
<>
{ toggleUI }
<RangeControl
value={ simulationWidth }
label={ __( 'Simulated width value' ) }
min={ 0 }
max={ 2000 }
allowReset
onChange={ ( newValue ) => ( setSimulationWidth( newValue ) ) }
/>
<__experimentalSimulateMediaQuery
width={ simulationWidth }
partialPaths={ PARTIAL_PATHS }
/>
</>
);
}

const SettingsSidebar = ( { sidebarName } ) => (
<Sidebar name={ sidebarName }>
<SettingsHeader sidebarName={ sidebarName } />
Expand All @@ -43,6 +82,7 @@ const SettingsSidebar = ( { sidebarName } ) => (
{ sidebarName === 'edit-post/block' && (
<BlockInspector />
) }
<TestSimulateMediaQuery />
</Panel>
</Sidebar>
);
Expand Down