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: Link to the block type panel when selecting a block with Styles open #49350

Merged
merged 7 commits into from
Apr 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@

## Unreleased

### Enhancements

- `DropZone`: Smooth animation ([#49517](https://github.com/WordPress/gutenberg/pull/49517)).
- `Navigator`: Add `skipFocus` property in `NavigateOptions`. ([#49350](https://github.com/WordPress/gutenberg/pull/49350)).

## 23.7.0 (2023-03-29)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ function UnconnectedNavigatorProvider(
const {
focusTargetSelector,
isBack = false,
skipFocus = false,
...restOptions
} = options;

Expand All @@ -168,6 +169,7 @@ function UnconnectedNavigatorProvider(
path,
isBack,
hasRestoredFocus: false,
skipFocus,
};

if ( prevLocationHistory.length < 1 ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ const animationExitDelay = 0;
// as some of them would overlap with HTML props (e.g. `onAnimationStart`, ...)
type Props = Omit<
WordPressComponentProps< NavigatorScreenProps, 'div', false >,
keyof MotionProps
Exclude< keyof MotionProps, 'style' >
>;

function UnconnectedNavigatorScreen(
Expand Down Expand Up @@ -100,11 +100,13 @@ function UnconnectedNavigatorScreen(
// - when the screen becomes visible
// - if the wrapper ref has been assigned
// - if focus hasn't already been restored for the current location
// - if the `skipFocus` option is not set to `true`. This is useful when we trigger the navigation outside of NavigatorScreen.
if (
isInitialLocation ||
! isMatch ||
! wrapperRef.current ||
locationRef.current.hasRestoredFocus
locationRef.current.hasRestoredFocus ||
location.skipFocus
Copy link
Contributor

Choose a reason for hiding this comment

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

let's say that we trigger a goTo navigation with skipFocus = true. At a later time, we navigate "back" (either via goBack, or via goToParent) to that same location. Should we skip focussing in that case too? Should we allow setting the the skipFocus specifically for those navigations too? Or should we never skip focus when navigating back?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably we could, but it's not a use case right now, so I think we could leave it for now. No strong opinions, but I don't think we would need a full 'external' navigation..

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good. Let's assume, at least for now, that navigating backwards will always restore focus.

) {
return;
}
Expand Down Expand Up @@ -143,6 +145,7 @@ function UnconnectedNavigatorScreen(
isMatch,
location.isBack,
location.focusTargetSelector,
location.skipFocus,
] );

const mergedWrapperRef = useMergeRefs( [ forwardedRef, wrapperRef ] );
Expand Down
68 changes: 68 additions & 0 deletions packages/components/src/navigator/stories/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -298,3 +298,71 @@ export const NestedNavigator: ComponentStory< typeof NavigatorProvider > =
NestedNavigator.args = {
initialPath: '/child2/grandchild',
};

const NavigatorButtonWithSkipFocus = ( {
path,
onClick,
...props
}: React.ComponentProps< typeof NavigatorButton > ) => {
const { goTo } = useNavigator();

return (
<Button
{ ...props }
onClick={ ( e: React.MouseEvent< HTMLButtonElement > ) => {
goTo( path, { skipFocus: true } );
onClick?.( e );
} }
/>
);
};

export const SkipFocus: ComponentStory< typeof NavigatorProvider > = (
args
) => {
return <NavigatorProvider { ...args } />;
};
SkipFocus.args = {
initialPath: '/',
children: (
<>
<div
style={ {
height: 250,
border: '1px solid black',
} }
>
<NavigatorScreen
path="/"
style={ {
height: '100%',
} }
>
<h1>Home screen</h1>
<NavigatorButton variant="secondary" path="/child">
Go to child screen.
</NavigatorButton>
</NavigatorScreen>
<NavigatorScreen
path="/child"
style={ {
height: '100%',
} }
>
<h2>Child screen</h2>
<NavigatorToParentButton variant="secondary">
Go to parent screen.
</NavigatorToParentButton>
</NavigatorScreen>
</div>

<NavigatorButtonWithSkipFocus
variant="secondary"
path="/child"
style={ { margin: '1rem 2rem' } }
>
Go to child screen, but keep focus on this button
</NavigatorButtonWithSkipFocus>
</>
),
};
52 changes: 52 additions & 0 deletions packages/components/src/navigator/test/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
NavigatorToParentButton,
useNavigator,
} from '..';
import type { NavigateOptions } from '../types';

const INVALID_HTML_ATTRIBUTE = {
raw: ' "\'><=invalid_path',
Expand Down Expand Up @@ -57,13 +58,15 @@ const BUTTON_TEXT = {
'Navigate to screen with an invalid HTML value as a path.',
back: 'Go back',
backUsingGoTo: 'Go back using goTo',
goToWithSkipFocus: 'Go to with skipFocus',
};

type CustomTestOnClickHandler = (
args:
| {
type: 'goTo';
path: string;
options?: NavigateOptions;
}
| { type: 'goBack' }
| { type: 'goToParent' }
Expand Down Expand Up @@ -108,6 +111,26 @@ function CustomNavigatorGoToBackButton( {
);
}

function CustomNavigatorGoToSkipFocusButton( {
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting — we can keep things as they are for now, but if we find ourselves having to re-create clones of NavigatorButton just to pass one specific option, we may consider exposing options as a prop.

No action required for now, but something to keep in mind for the future.

path,
onClick,
...props
}: Omit< ComponentPropsWithoutRef< typeof NavigatorButton >, 'onClick' > & {
onClick?: CustomTestOnClickHandler;
} ) {
const { goTo } = useNavigator();
return (
<Button
onClick={ () => {
goTo( path, { skipFocus: true } );
// Used to spy on the values passed to `navigator.goTo`.
onClick?.( { type: 'goTo', path } );
} }
{ ...props }
/>
);
}

function CustomNavigatorBackButton( {
onClick,
...props
Expand Down Expand Up @@ -342,6 +365,12 @@ const MyHierarchicalNavigation = ( {
{ BUTTON_TEXT.backUsingGoTo }
</CustomNavigatorGoToBackButton>
</NavigatorScreen>
<CustomNavigatorGoToSkipFocusButton
path={ PATHS.NESTED }
onClick={ onNavigatorButtonClick }
>
{ BUTTON_TEXT.goToWithSkipFocus }
</CustomNavigatorGoToSkipFocusButton>
</NavigatorProvider>
</>
);
Expand Down Expand Up @@ -716,6 +745,29 @@ describe( 'Navigator', () => {
await user.click( getNavigationButton( 'back' ) );
expect( getNavigationButton( 'toChildScreen' ) ).toHaveFocus();
} );

it( 'should skip focus based on location `skipFocus` option', async () => {
const user = userEvent.setup();
render( <MyHierarchicalNavigation /> );

// Navigate to child screen with skipFocus.
await user.click( getNavigationButton( 'goToWithSkipFocus' ) );
expect( queryScreen( 'home' ) ).not.toBeInTheDocument();
expect( getScreen( 'nested' ) ).toBeInTheDocument();

// The clicked button should remain focused.
expect( getNavigationButton( 'goToWithSkipFocus' ) ).toHaveFocus();

// Navigate back to parent screen.
await user.click( getNavigationButton( 'back' ) );
expect( getScreen( 'child' ) ).toBeInTheDocument();
// The first tabbable element receives focus.
expect(
screen.getByRole( 'button', {
name: 'First tabbable child screen button',
} )
).toHaveFocus();
} );
} );

describe( 'animation', () => {
Expand Down
3 changes: 2 additions & 1 deletion packages/components/src/navigator/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ import type { ButtonAsButtonProps } from '../button/types';

export type MatchParams = Record< string, string | string[] >;

type NavigateOptions = {
export type NavigateOptions = {
focusTargetSelector?: string;
isBack?: boolean;
skipFocus?: boolean;
};

export type NavigatorLocation = NavigateOptions & {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,21 +57,25 @@ function useSortedBlockTypes() {
return [ ...coreItems, ...nonCoreItems ];
}

function BlockMenuItem( { block } ) {
const [ rawSettings ] = useGlobalSetting( '', block.name );
const settings = useSettingsForBlockElement( rawSettings, block.name );
export function useBlockHasGlobalStyles( blockName ) {
const [ rawSettings ] = useGlobalSetting( '', blockName );
const settings = useSettingsForBlockElement( rawSettings, blockName );
const hasTypographyPanel = useHasTypographyPanel( settings );
const hasColorPanel = useHasColorPanel( settings );
const hasBorderPanel = useHasBorderPanel( settings );
const hasDimensionsPanel = useHasDimensionsPanel( settings );
const hasLayoutPanel = hasBorderPanel || hasDimensionsPanel;
const hasVariationsPanel = useHasVariationsPanel( block.name );
const hasBlockMenuItem =
const hasVariationsPanel = useHasVariationsPanel( blockName );
const hasGlobalStyles =
hasTypographyPanel ||
hasColorPanel ||
hasLayoutPanel ||
hasVariationsPanel;
return hasGlobalStyles;
}

function BlockMenuItem( { block } ) {
const hasBlockMenuItem = useBlockHasGlobalStyles( block.name );
if ( ! hasBlockMenuItem ) {
return null;
}
Expand Down
46 changes: 44 additions & 2 deletions packages/edit-site/src/components/global-styles/ui.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,24 @@ import {
} from '@wordpress/components';
import { getBlockTypes, store as blocksStore } from '@wordpress/blocks';
import { useSelect, useDispatch } from '@wordpress/data';
import { privateApis as blockEditorPrivateApis } from '@wordpress/block-editor';
import {
privateApis as blockEditorPrivateApis,
store as blockEditorStore,
} from '@wordpress/block-editor';
import { __ } from '@wordpress/i18n';
import { store as preferencesStore } from '@wordpress/preferences';
import { moreVertical } from '@wordpress/icons';
import { store as coreStore } from '@wordpress/core-data';
import { useEffect, useRef } from '@wordpress/element';

/**
* Internal dependencies
*/
import ScreenRoot from './screen-root';
import ScreenBlockList from './screen-block-list';
import {
useBlockHasGlobalStyles,
default as ScreenBlockList,
} from './screen-block-list';
import ScreenBlock from './screen-block';
import ScreenTypography from './screen-typography';
import ScreenTypographyElement from './screen-typography-element';
Expand Down Expand Up @@ -255,6 +262,40 @@ function GlobalStylesStyleBook( { onClose } ) {
);
}

function GlobalStylesBlockLink() {
const navigator = useNavigator();
const isMounted = useRef();
const { selectedBlockName, selectedBlockClientId } = useSelect(
( select ) => {
const { getSelectedBlockClientId, getBlockName } =
select( blockEditorStore );
const clientId = getSelectedBlockClientId();
return {
selectedBlockName: getBlockName( clientId ),
selectedBlockClientId: clientId,
};
},
[]
);
const blockHasGlobalStyles = useBlockHasGlobalStyles( selectedBlockName );
useEffect( () => {
// Avoid navigating to the block screen on mount.
if ( ! isMounted.current ) {
isMounted.current = true;
return;
}
if ( ! selectedBlockClientId || ! blockHasGlobalStyles ) {
return;
}
const path = '/blocks/' + encodeURIComponent( selectedBlockName );
// Avoid navigating to the same path. This can happen when selecting
// a new block of the same type.
if ( path !== navigator.location.path ) {
navigator.goTo( path, { skipFocus: true } );
}
}, [ selectedBlockClientId, selectedBlockName, blockHasGlobalStyles ] );
}

function GlobalStylesUI( { isStyleBookOpened, onCloseStyleBook } ) {
const blocks = getBlockTypes();

Expand Down Expand Up @@ -307,6 +348,7 @@ function GlobalStylesUI( { isStyleBookOpened, onCloseStyleBook } ) {
) }

<GlobalStylesActionMenu />
<GlobalStylesBlockLink />
</NavigatorProvider>
);
}
Expand Down