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

Allow styles to be changed dynamically through editor settings #52767

Merged
merged 4 commits into from
Jul 21, 2023
Merged

Allow styles to be changed dynamically through editor settings #52767

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b1c6526
Allow styles to be changed dynamically through editor settings
ellatrix Jul 19, 2023
01a9803
Simplify test
ellatrix Jul 19, 2023
00fde60
Filter styles coming from the EditorProvider, not before it's passed …
ellatrix Jul 19, 2023
53f4422
Add e2e test for theme styles
ellatrix Jul 19, 2023
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
16 changes: 16 additions & 0 deletions packages/e2e-tests/plugins/iframed-enqueue-block-editor-settings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php
/**
* Plugin Name: Gutenberg Test Iframed enqueue block editor settings
* Plugin URI: https://github.com/WordPress/gutenberg
* Author: Gutenberg Team
*
* @package gutenberg-test-iframed-iframed-enqueue-block-editor-settings
*/

add_action(
'block_editor_settings_all',
function( $settings ) {
$settings['styles'][] = array( 'css' => 'p { border: 1px solid red }' );
return $settings;
}
);
65 changes: 65 additions & 0 deletions packages/e2e-tests/specs/editor/plugins/iframed-enqueue-block-editor-settings.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/**
* WordPress dependencies
*/
import {
activatePlugin,
createNewPost,
deactivatePlugin,
canvas,
activateTheme,
} from '@wordpress/e2e-test-utils';

async function getComputedStyle( context, selector, property ) {
return await context.evaluate(
( sel, prop ) =>
window.getComputedStyle( document.querySelector( sel ) )[ prop ],
selector,
property
);
}

describe( 'iframed block editor settings styles', () => {
beforeEach( async () => {
// Activate the empty theme (block based theme), which is iframed.
await activateTheme( 'emptytheme' );
await activatePlugin(
'gutenberg-test-iframed-enqueue-block-editor-settings'
);
await createNewPost();
} );

afterEach( async () => {
await deactivatePlugin(
'gutenberg-test-iframed-enqueue-block-editor-settings'
);
await activateTheme( 'twentytwentyone' );
} );

it( 'should load styles added through block editor settings', async () => {
await page.waitForSelector( 'iframe[name="editor-canvas"]' );
// Expect a red border (added in PHP).
expect( await getComputedStyle( canvas(), 'p', 'border-color' ) ).toBe(
'rgb(255, 0, 0)'
);

await page.evaluate( () => {
const settings = window.wp.data
.select( 'core/editor' )
.getEditorSettings();
wp.data.dispatch( 'core/editor' ).updateEditorSettings( {
...settings,
styles: [
...settings.styles,
{
css: 'p { border-width: 2px; }',
},
],
} );
} );

// Expect a 2px border (added in JS).
expect( await getComputedStyle( canvas(), 'p', 'border-width' ) ).toBe(
'2px'
);
} );
} );
4 changes: 3 additions & 1 deletion packages/edit-post/src/components/layout/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ const interfaceLabels = {
footer: __( 'Editor footer' ),
};

function Layout( { styles } ) {
function Layout() {
const isMobileViewport = useViewportMatch( 'medium', '<' );
const isHugeViewport = useViewportMatch( 'huge', '>=' );
const isLargeViewport = useViewportMatch( 'large' );
Expand All @@ -89,6 +89,7 @@ function Layout( { styles } ) {
showBlockBreadcrumbs,
isTemplateMode,
documentLabel,
styles,
} = useSelect( ( select ) => {
const { getEditorSettings, getPostTypeLabel } = select( editorStore );
const editorSettings = getEditorSettings();
Expand Down Expand Up @@ -125,6 +126,7 @@ function Layout( { styles } ) {
),
// translators: Default label for the Document in the Block Breadcrumb.
documentLabel: postTypeLabel || _x( 'Document', 'noun' ),
styles: editorSettings.styles,
};
}, [] );

Expand Down
42 changes: 21 additions & 21 deletions packages/edit-post/src/editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,22 +143,6 @@ function Editor( { postId, postType, settings, initialEdits, ...props } ) {
);
}

return result;
}, [
settings,
hasFixedToolbar,
hasInlineToolbar,
focusMode,
isDistractionFree,
hiddenBlockTypes,
blockTypes,
preferredStyleVariations,
setIsInserterOpened,
updatePreferredStyleVariations,
keepCaretInsideBlock,
] );

const styles = useMemo( () => {
const themeStyles = [];
const presetStyles = [];
settings.styles?.forEach( ( style ) => {
Expand Down Expand Up @@ -191,10 +175,26 @@ function Editor( { postId, postType, settings, initialEdits, ...props } ) {
} );
}

return hasThemeStyles && themeStyles.length
? settings.styles
: defaultEditorStyles;
}, [ settings, hasThemeStyles ] );
result.styles =
hasThemeStyles && themeStyles.length
Copy link
Contributor

Choose a reason for hiding this comment

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

hasThemeStyles flag corresponds to a user preferences that is changeable from the preferences modal. That means that any change to the "settings" that is done by updateEditorSettings call from plugin authors will override any change that this preference might have.

Meaning we might want to avoid doing anything here to the "styles" property and instead move the logic of this preference to within the EditorProvider component (potentially need to make hasThemeStyles a new setting in editorSettings though).

Maybe there are other options, but it does raise the question about whether "styles" was ever meant to be "editable".

Copy link
Member Author

@ellatrix ellatrix Jul 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

You're right, the styles should be filtered after they are passed down to the EditorProvider. We can simply move the filtering to Layout, because that's a component within that provider. I fixed this in 00fde60. I can add an extra e2e test to ensure that this works correctly.

? settings.styles
: defaultEditorStyles;

return result;
}, [
settings,
hasFixedToolbar,
hasInlineToolbar,
focusMode,
isDistractionFree,
hiddenBlockTypes,
blockTypes,
preferredStyleVariations,
setIsInserterOpened,
updatePreferredStyleVariations,
keepCaretInsideBlock,
hasThemeStyles,
] );

if ( ! post ) {
return null;
Expand All @@ -214,7 +214,7 @@ function Editor( { postId, postType, settings, initialEdits, ...props } ) {
<ErrorBoundary>
<CommandMenu />
<EditorInitialization postId={ postId } />
<Layout styles={ styles } />
<Layout />
</ErrorBoundary>
<PostLockedModal />
</ExperimentalEditorProvider>
Expand Down