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

Navigator: Deprecate 36px default size for buttons #68330

Open
wants to merge 6 commits into
base: button/dropdown-menu
Choose a base branch
from
Open

Navigator: Deprecate 36px default size for buttons #68330

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f797dc3
Navigator: Deprecate 36px default size for buttons
mirka Dec 26, 2024
72f173a
Update docs
mirka Dec 26, 2024
9fc9a78
Add prop to stories
mirka Dec 26, 2024
7144d75
Only warn when `as` is undefined
mirka Dec 26, 2024
6232cd9
Add changelog
mirka Dec 26, 2024
3069986
Fix in PreferencesModalTabs (narrow viewport)
mirka Dec 26, 2024
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/components/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
- Soft deprecate `ButtonGroup` component. Use `ToggleGroupControl` instead ([#65429](https://github.com/WordPress/gutenberg/pull/65429)).
- `Navigation`: Log deprecation warning for removal in WP 7.1. Use `Navigator` instead ([#68158](https://github.com/WordPress/gutenberg/pull/68158)).
- `DropdownMenu`: Deprecate 36px default size for default toggle button ([#68329](https://github.com/WordPress/gutenberg/pull/68329)).
- `Navigator.Button`, `Navigator.BackButton`: Deprecate 36px default size ([#68330](https://github.com/WordPress/gutenberg/pull/68330)).

### Bug Fixes

Expand Down
6 changes: 4 additions & 2 deletions packages/components/src/navigator/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,15 @@ const MyNavigation = () => (
<Navigator initialPath="/">
<Navigator.Screen path="/">
<p>This is the home screen.</p>
<Navigator.Button path="/child">
<Navigator.Button path="/child" __next40pxDefaultSize>
Navigate to child screen.
</Navigator.Button>
</Navigator.Screen>
<Navigator.Screen path="/child">
<p>This is the child screen.</p>
<Navigator.BackButton>Go back</Navigator.BackButton>
<Navigator.BackButton __next40pxDefaultSize>
Go back
</Navigator.BackButton>
</Navigator.Screen>
</Navigator>
);
Expand Down
16 changes: 8 additions & 8 deletions packages/components/src/navigator/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@ export { useNavigator } from './use-navigator';
* <Navigator initialPath="/">
* <Navigator.Screen path="/">
* <p>This is the home screen.</p>
* <Navigator.Button path="/child">
* <Navigator.Button path="/child" __next40pxDefaultSize>
* Navigate to child screen.
* </Navigator.Button>
* </Navigator.Screen>
*
* <Navigator.Screen path="/child">
* <p>This is the child screen.</p>
* <Navigator.BackButton>
* <Navigator.BackButton __next40pxDefaultSize>
* Go back
* </Navigator.BackButton>
* </Navigator.Screen>
Expand All @@ -48,14 +48,14 @@ export const Navigator = Object.assign( TopLevelNavigator, {
* <Navigator initialPath="/">
* <Navigator.Screen path="/">
* <p>This is the home screen.</p>
* <Navigator.Button path="/child">
* <Navigator.Button path="/child" __next40pxDefaultSize>
* Navigate to child screen.
* </Navigator.Button>
* </Navigator.Screen>
*
* <Navigator.Screen path="/child">
* <p>This is the child screen.</p>
* <Navigator.BackButton>
* <Navigator.BackButton __next40pxDefaultSize>
* Go back
* </Navigator.BackButton>
* </Navigator.Screen>
Expand All @@ -79,14 +79,14 @@ export const Navigator = Object.assign( TopLevelNavigator, {
* <Navigator initialPath="/">
* <Navigator.Screen path="/">
* <p>This is the home screen.</p>
* <Navigator.Button path="/child">
* <Navigator.Button path="/child" __next40pxDefaultSize>
* Navigate to child screen.
* </Navigator.Button>
* </Navigator.Screen>
*
* <Navigator.Screen path="/child">
* <p>This is the child screen.</p>
* <Navigator.BackButton>
* <Navigator.BackButton __next40pxDefaultSize>
* Go back
* </Navigator.BackButton>
* </Navigator.Screen>
Expand All @@ -110,14 +110,14 @@ export const Navigator = Object.assign( TopLevelNavigator, {
* <Navigator initialPath="/">
* <Navigator.Screen path="/">
* <p>This is the home screen.</p>
* <Navigator.Button path="/child">
* <Navigator.Button path="/child" __next40pxDefaultSize>
* Navigate to child screen.
* </Navigator.Button>
* </Navigator.Screen>
*
* <Navigator.Screen path="/child">
* <p>This is the child screen.</p>
* <Navigator.BackButton>
* <Navigator.BackButton __next40pxDefaultSize>
* Go back
* </Navigator.BackButton>
* </Navigator.Screen>
Expand Down
15 changes: 13 additions & 2 deletions packages/components/src/navigator/navigator-back-button/hook.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,14 @@ import { useContextSystem } from '../../context';
import Button from '../../button';
import { useNavigator } from '../use-navigator';
import type { NavigatorBackButtonProps } from '../types';
import { maybeWarnDeprecated36pxSize } from '../../utils/deprecated-36px-size';

export function useNavigatorBackButton(
props: WordPressComponentProps< NavigatorBackButtonProps, 'button' >
) {
const {
onClick,
as = Button,
as,

...otherProps
} = useContextSystem( props, 'Navigator.BackButton' );
Expand All @@ -33,8 +34,18 @@ export function useNavigatorBackButton(
[ goBack, onClick ]
);

if ( as === undefined ) {
maybeWarnDeprecated36pxSize( {
componentName: 'Navigator.BackButton',
__next40pxDefaultSize: otherProps.__next40pxDefaultSize,
size: otherProps.size,
hint: 'Set the `__next40pxDefaultSize` prop to true to start opting into the new default size, which will become the default in a future version. For icon buttons, consider setting a non-default size like `size: "compact"`.',
} );
}

return {
as,
as: as ?? Button,
__shouldNotWarnDeprecated36pxSize: as === undefined,
Copy link
Member Author

Choose a reason for hiding this comment

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

This is to prevent a redundant warning from Button when Navigator.BackButton has already warned.

onClick: handleClick,
...otherProps,
};
Expand Down
15 changes: 13 additions & 2 deletions packages/components/src/navigator/navigator-button/hook.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { useContextSystem } from '../../context';
import Button from '../../button';
import { useNavigator } from '../use-navigator';
import type { NavigatorButtonProps } from '../types';
import { maybeWarnDeprecated36pxSize } from '../../utils/deprecated-36px-size';

const cssSelectorForAttribute = ( attrName: string, attrValue: string ) =>
`[${ attrName }="${ attrValue }"]`;
Expand All @@ -22,7 +23,7 @@ export function useNavigatorButton(
const {
path,
onClick,
as = Button,
as,
attributeName = 'id',
...otherProps
} = useContextSystem( props, 'Navigator.Button' );
Expand All @@ -45,8 +46,18 @@ export function useNavigatorButton(
[ goTo, onClick, attributeName, escapedPath ]
);

if ( as === undefined ) {
maybeWarnDeprecated36pxSize( {
componentName: 'Navigator.Button',
__next40pxDefaultSize: otherProps.__next40pxDefaultSize,
size: otherProps.size,
hint: 'Set the `__next40pxDefaultSize` prop to true to start opting into the new default size, which will become the default in a future version. For icon buttons, consider setting a non-default size like `size: "compact"`.',
} );
}

return {
as,
as: as ?? Button,
__shouldNotWarnDeprecated36pxSize: as === undefined,
onClick: handleClick,
...otherProps,
[ attributeName ]: escapedPath,
Expand Down
42 changes: 34 additions & 8 deletions packages/components/src/navigator/stories/index.story.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,27 @@ export const Default: StoryObj< typeof Navigator > = {
<h2>This is the home screen.</h2>

<VStack alignment="left">
<Navigator.Button variant="primary" path="/child">
<Navigator.Button
variant="primary"
path="/child"
__next40pxDefaultSize
>
Go to child screen.
</Navigator.Button>

<Navigator.Button variant="primary" path="/product/1">
<Navigator.Button
variant="primary"
path="/product/1"
__next40pxDefaultSize
>
Go to dynamic path screen with id 1.
</Navigator.Button>

<Navigator.Button variant="primary" path="/product/2">
<Navigator.Button
variant="primary"
path="/product/2"
__next40pxDefaultSize
>
Go to dynamic path screen with id 2.
</Navigator.Button>
</VStack>
Expand All @@ -82,13 +94,17 @@ export const Default: StoryObj< typeof Navigator > = {
<Navigator.Screen path="/child">
<h2>This is the child screen.</h2>
<HStack spacing={ 2 } alignment="left">
<Navigator.BackButton variant="secondary">
<Navigator.BackButton
variant="secondary"
__next40pxDefaultSize
>
Go back
</Navigator.BackButton>

<Navigator.Button
variant="primary"
path="/child/grandchild"
__next40pxDefaultSize
>
Go to grand child screen.
</Navigator.Button>
Expand All @@ -97,7 +113,10 @@ export const Default: StoryObj< typeof Navigator > = {

<Navigator.Screen path="/child/grandchild">
<h2>This is the grand child screen.</h2>
<Navigator.BackButton variant="secondary">
<Navigator.BackButton
variant="secondary"
__next40pxDefaultSize
>
Go back
</Navigator.BackButton>
</Navigator.Screen>
Expand All @@ -120,7 +139,7 @@ function DynamicScreen() {
This screen can parse params dynamically. The current id is:{ ' ' }
{ params.id }
</p>
<Navigator.BackButton variant="secondary">
<Navigator.BackButton variant="secondary" __next40pxDefaultSize>
Go back
</Navigator.BackButton>
</>
Expand Down Expand Up @@ -174,14 +193,21 @@ export const SkipFocus: StoryObj< typeof Navigator > = {
>
<Navigator.Screen path="/">
<h2>Home screen</h2>
<Navigator.Button variant="primary" path="/child">
<Navigator.Button
variant="primary"
path="/child"
__next40pxDefaultSize
>
Go to child screen.
</Navigator.Button>
</Navigator.Screen>

<Navigator.Screen path="/child">
<h2>Child screen</h2>
<Navigator.BackButton variant="secondary">
<Navigator.BackButton
variant="secondary"
__next40pxDefaultSize
>
Go back to home screen
</Navigator.BackButton>
</Navigator.Screen>
Expand Down
1 change: 1 addition & 0 deletions packages/preferences/src/components/preferences-modal-tabs/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ export default function PreferencesModalTabs( { sections } ) {
gap="6"
>
<Navigator.BackButton
size="compact"
Copy link
Member Author

Choose a reason for hiding this comment

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

In the block editor, open the Preferences modal in a narrow viewport.

Before After
Back button in Preferences modal, before Back button in Preferences modal, after

icon={
isRTL()
? chevronRight
Expand Down
Loading