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

Extract utilities to @wordpress/editor and @wordpress/router #63849

Closed
wants to merge 9 commits into from
Closed

Extract utilities to @wordpress/editor and @wordpress/router #63849

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
24950f2
Absorb Media component into post-fields
oandregal Jul 23, 2024
f740866
Absorb dataviews constants into post-fields
oandregal Jul 23, 2024
d0b0923
Add TODO: clarify what to do with wp_theme_preview parameter
oandregal Jul 23, 2024
2659079
Add TODO about view action
oandregal Jul 23, 2024
e4ee197
Add isPreviewingTheme, currentlyPreviewingTheme, Link, useLink to @wo…
oandregal Jul 23, 2024
14f766b
Update dependencies
oandregal Jul 23, 2024
6dcfe7b
Move usePostFields as a private API of editor package
oandregal Jul 23, 2024
eb9d3ee
Post actions: use shared field definition
oandregal Jul 23, 2024
61bd767
View action: respect wp_theme_preview parameter
oandregal Jul 23, 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
2 changes: 2 additions & 0 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions packages/edit-site/src/components/editor/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import { unlock } from '../../lock-unlock';
import { useSpecificEditorSettings } from '../block-editor/use-site-editor-settings';
import PluginTemplateSettingPanel from '../plugin-template-setting-panel';
import GlobalStylesSidebar from '../global-styles-sidebar';
import { isPreviewingTheme } from '../../utils/is-previewing-theme';
import {
getEditorCanvasContainerTitleAndIcon,
useHasEditorCanvasContainer,
Expand All @@ -48,7 +47,8 @@ import useEditorTitle from './use-editor-title';
import { useIsSiteEditorLoading } from '../layout/hooks';

const { Editor, BackButton } = unlock( editorPrivateApis );
const { useHistory, useLocation } = unlock( routerPrivateApis );
const { useHistory, useLocation, isPreviewingTheme } =
youknowriad marked this conversation as resolved.
Show resolved Hide resolved
unlock( routerPrivateApis );
const { BlockKeyboardShortcuts } = unlock( blockLibraryPrivateApis );

export default function EditSiteEditor( { isPostsList = false } ) {
Expand Down
23 changes: 0 additions & 23 deletions packages/edit-site/src/components/media/index.js
View file
Open in desktop

This file was deleted.

3 changes: 1 addition & 2 deletions packages/edit-site/src/components/page-patterns/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ import usePatternSettings from './use-pattern-settings';
import { unlock } from '../../lock-unlock';
import usePatterns from './use-patterns';
import PatternsHeader from './header';
import { useLink } from '../routes/link';
import { useAddedBy } from '../page-templates/hooks';
import { useEditPostAction } from '../dataviews-actions';
import { defaultGetTitle } from './search-items';
Expand All @@ -55,7 +54,7 @@ const { ExperimentalBlockEditorProvider, useGlobalStyle } = unlock(
blockEditorPrivateApis
);
const { usePostActions } = unlock( editorPrivateApis );
const { useLocation } = unlock( routerPrivateApis );
const { useLocation, useLink } = unlock( routerPrivateApis );

const EMPTY_ARRAY = [];
const defaultLayouts = {
Expand Down
5 changes: 1 addition & 4 deletions packages/edit-site/src/components/page-templates/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import {
*/
import { Async } from '../async';
import Page from '../page';
import { default as Link, useLink } from '../routes/link';
import AddNewTemplate from '../add-new-template';
import { useAddedBy } from './hooks';
import {
Expand All @@ -38,15 +37,13 @@ import {
LAYOUT_TABLE,
LAYOUT_LIST,
} from '../../utils/constants';

import usePatternSettings from '../page-patterns/use-pattern-settings';
import { unlock } from '../../lock-unlock';
import { useEditPostAction } from '../dataviews-actions';

const { usePostActions } = unlock( editorPrivateApis );

const { useGlobalStyle } = unlock( blockEditorPrivateApis );
const { useHistory, useLocation } = unlock( routerPrivateApis );
const { useHistory, useLocation, Link, useLink } = unlock( routerPrivateApis );

const EMPTY_ARRAY = [];

Expand Down
5 changes: 4 additions & 1 deletion packages/edit-site/src/components/post-edit/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,15 @@ import { useDispatch, useSelect } from '@wordpress/data';
import { store as coreDataStore } from '@wordpress/core-data';
import { Button } from '@wordpress/components';
import { useState, useMemo } from '@wordpress/element';
import { privateApis as editorPrivateApis } from '@wordpress/editor';

/**
* Internal dependencies
*/
import { unlock } from '../../lock-unlock';
import Page from '../page';
import usePostFields from '../post-fields';

const { usePostFields } = unlock( editorPrivateApis );

function PostEditForm( { postType, postId } ) {
const { item } = useSelect(
Expand Down
5 changes: 2 additions & 3 deletions packages/edit-site/src/components/post-list/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { DataViews } from '@wordpress/dataviews';
import { privateApis as editorPrivateApis } from '@wordpress/editor';
import { __ } from '@wordpress/i18n';
import { drawerRight } from '@wordpress/icons';
import { usePrevious } from '@wordpress/compose';

/**
* Internal dependencies
Expand All @@ -28,10 +29,8 @@ import {
import AddNewPostModal from '../add-new-post';
import { unlock } from '../../lock-unlock';
import { useEditPostAction } from '../dataviews-actions';
import { usePrevious } from '@wordpress/compose';
import usePostFields from '../post-fields';

const { usePostActions } = unlock( editorPrivateApis );
const { usePostActions, usePostFields } = unlock( editorPrivateApis );
const { useLocation, useHistory } = unlock( routerPrivateApis );
const EMPTY_ARRAY = [];

Expand Down
7 changes: 2 additions & 5 deletions packages/edit-site/src/components/save-button/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,10 @@ import {
* Internal dependencies
*/
import { store as editSiteStore } from '../../store';
import {
currentlyPreviewingTheme,
isPreviewingTheme,
} from '../../utils/is-previewing-theme';
import { unlock } from '../../lock-unlock';

const { useLocation } = unlock( routerPrivateApis );
const { useLocation, currentlyPreviewingTheme, isPreviewingTheme } =
unlock( routerPrivateApis );

export default function SaveButton( {
className = 'edit-site-save-button__button',
Expand Down
5 changes: 4 additions & 1 deletion packages/edit-site/src/components/save-hub/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,15 @@ import { useSelect } from '@wordpress/data';
import { __experimentalHStack as HStack } from '@wordpress/components';
import { store as coreStore } from '@wordpress/core-data';
import { check } from '@wordpress/icons';
import { privateApis as routerPrivateApis } from '@wordpress/router';

/**
* Internal dependencies
*/
import SaveButton from '../save-button';
import { isPreviewingTheme } from '../../utils/is-previewing-theme';
import { unlock } from '../../lock-unlock';

const { isPreviewingTheme } = unlock( routerPrivateApis );

export default function SaveHub() {
const { isDisabled, isSaving } = useSelect( ( select ) => {
Expand Down
4 changes: 3 additions & 1 deletion packages/edit-site/src/components/save-panel/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
import { useDispatch, useSelect } from '@wordpress/data';
import { __, sprintf } from '@wordpress/i18n';
import { store as coreStore } from '@wordpress/core-data';
import { privateApis as routerPrivateApis } from '@wordpress/router';

/**
* Internal dependencies
Expand All @@ -23,7 +24,8 @@ import { store as editSiteStore } from '../../store';
import { unlock } from '../../lock-unlock';
import { useActivateTheme } from '../../utils/use-activate-theme';
import { useActualCurrentTheme } from '../../utils/use-actual-current-theme';
import { isPreviewingTheme } from '../../utils/is-previewing-theme';

const { isPreviewingTheme } = unlock( routerPrivateApis );

const { EntitiesSavedStatesExtensible, NavigableRegion } =
unlock( privateApis );
Expand Down
3 changes: 1 addition & 2 deletions packages/edit-site/src/components/sidebar-dataviews/dataview-item.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,9 @@ import { VIEW_LAYOUTS } from '@wordpress/dataviews';
/**
* Internal dependencies
*/
import { useLink } from '../routes/link';
import SidebarNavigationItem from '../sidebar-navigation-item';
import { unlock } from '../../lock-unlock';
const { useLocation } = unlock( routerPrivateApis );
const { useLocation, useLink } = unlock( routerPrivateApis );

export default function DataViewItem( {
title,
Expand Down
6 changes: 4 additions & 2 deletions packages/edit-site/src/components/sidebar-navigation-screen-navigation-menus/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,21 @@ import {
Spinner,
} from '@wordpress/components';
import { navigation } from '@wordpress/icons';
import { privateApis as routerPrivateApis } from '@wordpress/router';

/**
* Internal dependencies
*/
import { unlock } from '../../lock-unlock';
import SidebarNavigationScreen from '../sidebar-navigation-screen';
import SidebarNavigationItem from '../sidebar-navigation-item';
import { PRELOADED_NAVIGATION_MENUS_QUERY } from './constants';
import { useLink } from '../routes/link';
import SingleNavigationMenu from '../sidebar-navigation-screen-navigation-menu/single-navigation-menu';
import useNavigationMenuHandlers from '../sidebar-navigation-screen-navigation-menu/use-navigation-menu-handlers';
import { unlock } from '../../lock-unlock';
import { NAVIGATION_POST_TYPE } from '../../utils/constants';

const { useLink } = unlock( routerPrivateApis );

// Copied from packages/block-library/src/navigation/edit/navigation-menu-selector.js.
function buildMenuLabel( title, id, status ) {
if ( ! title ) {
Expand Down
9 changes: 8 additions & 1 deletion packages/edit-site/src/components/sidebar-navigation-screen-patterns/category-item.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,22 @@
/**
* WordPress dependencies
*/
import { privateApis as routerPrivateApis } from '@wordpress/router';

/**
* Internal dependencies
*/
import { unlock } from '../../lock-unlock';
import SidebarNavigationItem from '../sidebar-navigation-item';
import { useLink } from '../routes/link';
import {
TEMPLATE_PART_POST_TYPE,
TEMPLATE_PART_ALL_AREAS_CATEGORY,
PATTERN_DEFAULT_CATEGORY,
PATTERN_TYPES,
} from '../../utils/constants';

const { useLink } = unlock( routerPrivateApis );

export default function CategoryItem( {
count,
icon,
Expand Down
7 changes: 2 additions & 5 deletions packages/edit-site/src/components/sidebar-navigation-screen/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,10 @@ import { useContext } from '@wordpress/element';
import { store as editSiteStore } from '../../store';
import { unlock } from '../../lock-unlock';
import SidebarButton from '../sidebar-button';
import {
isPreviewingTheme,
currentlyPreviewingTheme,
} from '../../utils/is-previewing-theme';
import { SidebarNavigationContext } from '../sidebar';

const { useHistory, useLocation } = unlock( routerPrivateApis );
const { useHistory, useLocation, isPreviewingTheme, currentlyPreviewingTheme } =
unlock( routerPrivateApis );

export default function SidebarNavigationScreen( {
isRoot,
Expand Down
3 changes: 1 addition & 2 deletions packages/edit-site/src/hooks/commands/use-edit-mode-commands.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,8 @@ import isTemplateRemovable from '../../utils/is-template-removable';
import isTemplateRevertable from '../../utils/is-template-revertable';
import { unlock } from '../../lock-unlock';
import { TEMPLATE_POST_TYPE } from '../../utils/constants';
import { useLink } from '../../components/routes/link';

const { useHistory } = unlock( routerPrivateApis );
const { useHistory, useLink } = unlock( routerPrivateApis );

function usePageContentFocusCommands() {
const { record: template } = useEditedEntityRecord();
Expand Down
7 changes: 2 additions & 5 deletions packages/edit-site/src/utils/use-activate-theme.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,9 @@ import { privateApis as routerPrivateApis } from '@wordpress/router';
* Internal dependencies
*/
import { unlock } from '../lock-unlock';
import {
isPreviewingTheme,
currentlyPreviewingTheme,
} from './is-previewing-theme';

const { useHistory } = unlock( routerPrivateApis );
const { useHistory, isPreviewingTheme, currentlyPreviewingTheme } =
unlock( routerPrivateApis );

/**
* This should be refactored to use the REST API, once the REST API can activate themes.
Expand Down
1 change: 1 addition & 0 deletions packages/editor/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
"@wordpress/private-apis": "file:../private-apis",
"@wordpress/reusable-blocks": "file:../reusable-blocks",
"@wordpress/rich-text": "file:../rich-text",
"@wordpress/router": "file:../router",
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 the major concern I have to land this PR.

By doing this, the @wordpress/editor package would depend on some utilities provided by the router that require a <RouterProvider> in the component tree. Is this something the editor package cares about? Or is it the higher-up edit-site/edit-post/etc. responsibility instead? @youknowriad I presume you have thoughts about this and can share advice.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a bit of a gray area.

If I understand properly, you want to move fields to "editor" (or core-data) or something like that. That's totally fine by me assuming all the fields are independent of the place they are used in (post editor, site editor...).

The problem is that the urls them selves are specific, in the site editor you want to navigate to some places but not in the post editor... Maybe for a start we can use always the same url and it should work regardless of where you are.

The other problem is that I'm not sure "urls" make sense in fields at all. I think for instance rendering "Media" or "Title" field or something shouldn't actually render an application url/link automatically. That feels like a separate config that should be passed to DataViews. For instance a getItemLink just like we have getItemId.

Also nothing that we already have something like getItemLabel but it's not implemented properly, right now we guess it by fetching the "primary" field which IMO is mixing separate concerns.

Copy link
Member Author

@oandregal oandregal Jul 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

If I understand properly, you want to move fields to "editor" (or core-data) or something like that.

Yeah. One of the issues I want to fix is: how do we share the field definition? Fields need to be available in three places now: edit-site/post-edit, edit-site/post-list, editor/post-actions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, just like the actions.

My goal for the actions was to move them one by one (and type them) into editor/src/dataviews/actions and I can see the same for fields in editor/src/dataviews/fields.

I think ultimately, it makes sense for editor/src/dataviews to be its own package: core-dataviews or something or actually be part of core-data

"@wordpress/server-side-render": "file:../server-side-render",
"@wordpress/url": "file:../url",
"@wordpress/warning": "file:../warning",
Expand Down
24 changes: 12 additions & 12 deletions packages/editor/src/components/post-actions/actions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
__experimentalVStack as VStack,
__experimentalNumberControl as NumberControl,
} from '@wordpress/components';
import { privateApis as routerPrivateApis } from '@wordpress/router';

/**
* Internal dependencies
Expand All @@ -34,21 +35,13 @@ import { store as editorStore } from '../../store';
import { unlock } from '../../lock-unlock';
import { CreateTemplatePartModalContents } from '../create-template-part-modal';
import { getItemTitle } from '../../dataviews/actions/utils';
import usePostFields from '../post-fields';

// Patterns.
const { PATTERN_TYPES, CreatePatternModalContents, useDuplicatePatternProps } =
unlock( patternsPrivateApis );

// TODO: this should be shared with other components (page-pages).
const fields = [
{
type: 'text',
header: __( 'Title' ),
id: 'title',
placeholder: __( 'No title' ),
getValue: ( { item } ) => item.title,
},
];
const { isPreviewingTheme, currentlyPreviewingTheme } =
unlock( routerPrivateApis );

const form = {
visibleFields: [ 'title' ],
Expand Down Expand Up @@ -483,7 +476,13 @@ const viewPostAction = {
},
callback( posts, { onActionPerformed } ) {
const post = posts[ 0 ];
window.open( post.link, '_blank' );
let url = post.link;
if ( isPreviewingTheme() ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this action, the reason you had to move previewing theme state to router?

Copy link
Member Author

@oandregal oandregal Jul 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Also because the useLink/Link components have them as a dependency. If I move those to the @wordpress/router, it cannot depend on @wordpress/edit-site (where isPreviewingTheme/etc. would live). See what I know about the wp_theme_preview parameter at #63849 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe in addition to the idea I shared here #63849 (comment) the site editor can override this action.

Copy link
Member Author

Choose a reason for hiding this comment

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

See conversation at https://github.com/WordPress/gutenberg/pull/63849/files#r1687870181

url = addQueryArgs( url, {
wp_theme_preview: currentlyPreviewingTheme(),
} );
}
window.open( url, '_blank' );
if ( onActionPerformed ) {
onActionPerformed( posts );
}
Expand Down Expand Up @@ -770,6 +769,7 @@ const useDuplicatePostAction = ( postType ) => {
),
} );

const { fields } = usePostFields();
const [ isCreatingPage, setIsCreatingPage ] =
useState( false );

Expand Down
Loading
Loading