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

Global Styles: Invoke zoomed-out view when selecting a style variation #44987

Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,22 @@ import classnames from 'classnames';
* WordPress dependencies
*/
import { store as coreStore } from '@wordpress/core-data';
import { useSelect } from '@wordpress/data';
import { useMemo, useContext, useState } from '@wordpress/element';
import { useSelect, useDispatch } from '@wordpress/data';
import {
useMemo,
useContext,
useState,
useEffect,
useRef,
} from '@wordpress/element';
import { ENTER } from '@wordpress/keycodes';
import {
__experimentalGrid as Grid,
Card,
CardBody,
} from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import { store as blockEditorStore } from '@wordpress/block-editor';

/**
* Internal dependencies
Expand Down Expand Up @@ -96,12 +103,14 @@ function Variation( { variation } ) {
}

function ScreenStyleVariations() {
const { variations } = useSelect( ( select ) => {
const { variations, mode } = useSelect( ( select ) => {
return {
variations:
select(
coreStore
).__experimentalGetCurrentThemeGlobalStylesVariations(),

mode: select( blockEditorStore ).__unstableGetEditorMode(),
};
}, [] );

Expand All @@ -120,6 +129,31 @@ function ScreenStyleVariations() {
];
}, [ variations ] );

const { __unstableSetEditorMode } = useDispatch( blockEditorStore );
const shouldRevertInitialMode = useRef( null );
useEffect( () => {
Copy link
Member

@oandregal oandregal Oct 17, 2022

Choose a reason for hiding this comment

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

We're tracking mode changes to decide whether to revert to the initial mode or not. If the user has done any mode changes since the style variations were opened, we won't revert to the initial.

So, in a situation like this:

  • user starts in edit mode
  • goes to style variations, so the editor is switched automatically to zoom-out mode
  • user intentionally switches off zoom-out mode (goes back to edit)
  • user intentionally switches on zoom-out mode
  • user closes global styles sidebar

What's the expected mode at this point?

With this PR, the mode stays in zoom-out. I think it makes sense because trying to be too smart may be too intrusive. Though, I see how switching off zoom-out mode can be something people do if they want more focus for a specific change, and then going back to the zoom-out. In that flow, staying in zoom-out after closing style variations may be weird.

Copy link
Member Author

Choose a reason for hiding this comment

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

I opted for this approach because it was the most simple code and seemed to fit most use cases. If you prefer, I can try an approach where if the end mode is still zoom-out we always revert to the initial mode.

// ignore changes to zoom-out mode as we explictily change to it on mount.
if ( mode !== 'zoom-out' ) {
shouldRevertInitialMode.current = false;
}
}, [ mode ] );

// Intentionality left without any dependency.
// This effect should only run the first time the component is rendered.
// The effect opens the zoom-out view if it is not open before when applying a style variation.
useEffect( () => {
if ( mode !== 'zoom-out' ) {
__unstableSetEditorMode( 'zoom-out' );
shouldRevertInitialMode.current = true;
return () => {
// if there were not mode changes revert to the initial mode when unmounting.
if ( shouldRevertInitialMode.current ) {
__unstableSetEditorMode( mode );
}
};
}
}, [] );

return (
<>
<ScreenHeader
Expand Down