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

Prevent distracting focused back button on site editor load #48472

Merged
merged 1 commit into from
Feb 27, 2023
Merged

Prevent distracting focused back button on site editor load #48472

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 11 additions & 0 deletions packages/edit-site/src/components/sidebar-navigation-screen/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,18 @@
.edit-site-sidebar-navigation-screen__back {
color: $gray-200;

// Focus (resets default button focus and use focus-visible).
&:focus:not(:disabled) {
box-shadow: none;
outline: none;
}
&:focus-visible:not(:disabled) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wonder at what point we could consider this globally for all buttons and not just this override. cc @ciampo

Copy link
Contributor

Choose a reason for hiding this comment

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

That would be a nice improvement, and in general I'd be up for bringing it to the source component.

The only challenge would be to make sure that there are no regressions when introducing the change. Button is a very complex component and it's not easy to grasp the extend of the consequences when making a change.

Would you be able to open a separate issue about this?

box-shadow: 0 0 0 var(--wp-admin-border-width-focus) var(--wp-components-color-accent, var(--wp-admin-theme-color, #007cba));
outline: 3px solid transparent;
}

&:hover,
&:focus-visible,
&:focus,
&:not([aria-disabled="true"]):active {
color: $white;
Expand Down
12 changes: 9 additions & 3 deletions packages/edit-site/src/components/sidebar/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* WordPress dependencies
*/
import { memo } from '@wordpress/element';
import { memo, useRef } from '@wordpress/element';
import {
__experimentalNavigatorProvider as NavigatorProvider,
__experimentalNavigatorScreen as NavigatorScreen,
Expand All @@ -13,11 +13,14 @@ import {
import SidebarNavigationScreenMain from '../sidebar-navigation-screen-main';
import SidebarNavigationScreenTemplates from '../sidebar-navigation-screen-templates';
import SidebarNavigationScreenTemplate from '../sidebar-navigation-screen-template';
import useSyncPathWithURL from '../sync-state-with-url/use-sync-path-with-url';
import useSyncPathWithURL, {
getPathFromURL,
} from '../sync-state-with-url/use-sync-path-with-url';
import SidebarNavigationScreenNavigationMenus from '../sidebar-navigation-screen-navigation-menus';
import SidebarNavigationScreenTemplatesBrowse from '../sidebar-navigation-screen-templates-browse';
import SaveButton from '../save-button';
import SidebarNavigationScreenNavigationItem from '../sidebar-navigation-screen-navigation-item';
import { useLocation } from '../routes';

function SidebarScreens() {
useSyncPathWithURL();
Expand Down Expand Up @@ -47,11 +50,14 @@ function SidebarScreens() {
}

function Sidebar() {
const { params: urlParams } = useLocation();
const initialPath = useRef( getPathFromURL( urlParams ) );

return (
<>
<NavigatorProvider
className="edit-site-sidebar__content"
initialPath="/"
initialPath={ initialPath.current }
>
<SidebarScreens />
</NavigatorProvider>
Expand Down
60 changes: 30 additions & 30 deletions packages/edit-site/src/components/sync-state-with-url/use-sync-path-with-url.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,34 @@ import { useEffect, useRef } from '@wordpress/element';
*/
import { useLocation, useHistory } from '../routes';

export function getPathFromURL( urlParams ) {
let path = urlParams?.path ?? '/';

// Compute the navigator path based on the URL params.
if (
urlParams?.postType &&
urlParams?.postId &&
// This is just a special case to support old WP versions that perform redirects.
// This code should be removed when we minimum WP version becomes 6.2.
urlParams?.postId !== 'none'
) {
switch ( urlParams.postType ) {
case 'wp_template':
case 'wp_template_part':
path = `/${ encodeURIComponent(
urlParams.postType
) }/${ encodeURIComponent( urlParams.postId ) }`;
break;
default:
path = `/navigation/${ encodeURIComponent(
urlParams.postType
) }/${ encodeURIComponent( urlParams.postId ) }`;
}
}

return path;
}

export default function useSyncPathWithURL() {
const history = useHistory();
const { params: urlParams } = useLocation();
Expand All @@ -18,13 +46,9 @@ export default function useSyncPathWithURL() {
goTo,
} = useNavigator();
const currentUrlParams = useRef( urlParams );
const currentPath = useRef();
const currentPath = useRef( navigatorLocation.path );

useEffect( () => {
// Don't trust the navigator path on initial render.
if ( currentPath.current === null ) {
return;
}
function updateUrlParams( newUrlParams ) {
if (
Object.entries( newUrlParams ).every( ( [ key, value ] ) => {
Expand Down Expand Up @@ -64,34 +88,10 @@ export default function useSyncPathWithURL() {

useEffect( () => {
currentUrlParams.current = urlParams;
let path = urlParams?.path ?? '/';

// Compute the navigator path based on the URL params.
if (
urlParams?.postType &&
urlParams?.postId &&
// This is just a special case to support old WP versions that perform redirects.
// This code should be removed when we minimum WP version becomes 6.2.
urlParams?.postId !== 'none'
) {
switch ( urlParams.postType ) {
case 'wp_template':
case 'wp_template_part':
path = `/${ encodeURIComponent(
urlParams.postType
) }/${ encodeURIComponent( urlParams.postId ) }`;
break;
default:
path = `/navigation/${ encodeURIComponent(
urlParams.postType
) }/${ encodeURIComponent( urlParams.postId ) }`;
}
}

const path = getPathFromURL( urlParams );
if ( currentPath.current !== path ) {
currentPath.current = path;
goTo( path );
}
goTo( path );
}, [ urlParams, goTo ] );
}