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

Block editor settings: delay selecting pageOnFront, pageForPosts #57290

Open
wants to merge 10 commits into
base: trunk
Choose a base branch
from
Open

Block editor settings: delay selecting pageOnFront, pageForPosts #57290

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
99f955b
Block editor settings: delay selecting pageOnFront, pageForPosts
ellatrix Dec 20, 2023
8d05001
Check if usePageSettings exists
ellatrix Dec 20, 2023
a9813b6
Use Symbol instead of experimental prefix
ellatrix Dec 21, 2023
3991a70
Add private keys for native
ellatrix Dec 21, 2023
ac45b44
use new key for all editors
ellatrix Dec 21, 2023
80a3e54
Extract useHandleEntitySearch
ellatrix Dec 21, 2023
cc80345
Absorb fetchLinkSuggestions
ellatrix Dec 21, 2023
15f78d9
Move hook to core-data
ellatrix Dec 21, 2023
17857cd
Move back creation result
ellatrix Dec 21, 2023
91dc709
Restore __experimentalFetchLinkSuggestions setting
ellatrix Dec 21, 2023
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
13 changes: 8 additions & 5 deletions packages/block-editor/src/components/link-control/use-search-handler.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,19 +108,22 @@ export default function useSearchHandler(
allowDirectEntry,
withCreateSuggestion
) {
const { fetchSearchSuggestions, pageOnFront, pageForPosts } = useSelect(
const { fetchSearchSuggestions, usePageSettings } = useSelect(
( select ) => {
const { getSettings } = select( blockEditorStore );

const settings = getSettings();
return {
pageOnFront: getSettings().pageOnFront,
pageForPosts: getSettings().pageForPosts,
usePageSettings: settings.__experimentalUsePageSettings,
fetchSearchSuggestions:
getSettings().__experimentalFetchLinkSuggestions,
settings.__experimentalFetchLinkSuggestions,
};
},
[]
);
// The function should either be undefined or a stable function reference
// throughout the editor lifetime, much like importing a function from a
// module. Maybe warn if this becomes a common pattern and it does change?
const { pageOnFront, pageForPosts } = usePageSettings?.() ?? {};
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 the only place where we need these settings? It seems we only use them in a callback, so maybe a better settings would be a callback getSiteSettings or something like that no?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also something else, I've noted, it seems that the "link control" component relies on a lot of settings (__experimentalFetchLinkSuggestions) for instance and all these settings are not used elsewhere (unless I'm wrong). So maybe it's the way we're implementing the link control that is wrong, maybe we should just have a "default link control" and make it a filterable component like "mediaUpload". (Just thinking of alternatives, I'm not sure which is best)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I think we should consider that too. I'm however not familiar enough with it to want to attempt a complete rewrite 😄

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'm trying to make things better step by step and make sure are the dependencies are completely private so it's easier to rewrite in the future

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems we only use them in a callback, so maybe a better settings would be a callback getSiteSettings or something like that no?

No, because it's used for rendered UI, not a callback. The selector is a resolver, so the component should re-render when the settings come in.

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems like useSearchHandler could entirely be a setting, but I'd prefer to attempt this separately because it would mean duplicating all this code to edit-widgets.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also something else, I've noted, it seems that the "link control" component relies on a lot of settings (__experimentalFetchLinkSuggestions) for instance and all these settings are not used elsewhere (unless I'm wrong). So maybe it's the way we're implementing the link control that is wrong,

@youknowriad I'd be grateful to hear more about how you'd like LinkControl to change. Now is the time because a lot of demands are being placed on it.

...maybe we should just have a "default link control" and make it a filterable component like "mediaUpload". (Just thinking of alternatives, I'm not sure which is best)

I'd like to understand more about this.


const directEntryHandler = allowDirectEntry
? handleDirectEntry
Expand Down
68 changes: 35 additions & 33 deletions packages/edit-widgets/src/components/widget-areas-block-editor-provider/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,43 +27,48 @@ import { unlock } from '../../lock-unlock';

const { ExperimentalBlockEditorProvider } = unlock( blockEditorPrivateApis );
const { PatternsMenuItems } = unlock( editPatternsPrivateApis );
export default function WidgetAreasBlockEditorProvider( {
blockEditorSettings,
children,
...props
} ) {
const mediaPermissions = useResourcePermissions( 'media' );
const isLargeViewport = useViewportMatch( 'medium' );
const {
reusableBlocks,
isFixedToolbarActive,
keepCaretInsideBlock,
pageOnFront,
pageForPosts,
} = useSelect( ( select ) => {
const { canUser, getEntityRecord, getEntityRecords } =
select( coreStore );

function usePageSettings() {
return useSelect( ( select ) => {
const { canUser, getEntityRecord } = select( coreStore );

const siteSettings = canUser( 'read', 'settings' )
? getEntityRecord( 'root', 'site' )
: undefined;

return {
widgetAreas: select( editWidgetsStore ).getWidgetAreas(),
widgets: select( editWidgetsStore ).getWidgets(),
reusableBlocks: ALLOW_REUSABLE_BLOCKS
? getEntityRecords( 'postType', 'wp_block' )
: [],
isFixedToolbarActive: !! select( preferencesStore ).get(
'core/edit-widgets',
'fixedToolbar'
),
keepCaretInsideBlock: !! select( preferencesStore ).get(
'core/edit-widgets',
'keepCaretInsideBlock'
),
pageOnFront: siteSettings?.page_on_front,
pageForPosts: siteSettings?.page_for_posts,
};
}, [] );
}

export default function WidgetAreasBlockEditorProvider( {
blockEditorSettings,
children,
...props
} ) {
const mediaPermissions = useResourcePermissions( 'media' );
const isLargeViewport = useViewportMatch( 'medium' );
const { reusableBlocks, isFixedToolbarActive, keepCaretInsideBlock } =
useSelect( ( select ) => {
const { getEntityRecords } = select( coreStore );
return {
widgetAreas: select( editWidgetsStore ).getWidgetAreas(),
widgets: select( editWidgetsStore ).getWidgets(),
reusableBlocks: ALLOW_REUSABLE_BLOCKS
? getEntityRecords( 'postType', 'wp_block' )
: [],
isFixedToolbarActive: !! select( preferencesStore ).get(
'core/edit-widgets',
'fixedToolbar'
),
keepCaretInsideBlock: !! select( preferencesStore ).get(
'core/edit-widgets',
'keepCaretInsideBlock'
),
};
}, [] );
const { setIsInserterOpened } = useDispatch( editWidgetsStore );

const settings = useMemo( () => {
Expand All @@ -85,8 +90,7 @@ export default function WidgetAreasBlockEditorProvider( {
mediaUpload: mediaUploadBlockEditor,
templateLock: 'all',
__experimentalSetIsInserterOpened: setIsInserterOpened,
pageOnFront,
pageForPosts,
__experimentalUsePageSettings: usePageSettings,
};
}, [
blockEditorSettings,
Expand All @@ -96,8 +100,6 @@ export default function WidgetAreasBlockEditorProvider( {
mediaPermissions.canCreate,
reusableBlocks,
setIsInserterOpened,
pageOnFront,
pageForPosts,
] );

const widgetAreaId = useLastSelectedWidgetArea();
Expand Down
29 changes: 16 additions & 13 deletions packages/editor/src/components/provider/use-block-editor-settings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,21 @@ const BLOCK_EDITOR_SETTINGS = [
'__experimentalArchiveTitleNameLabel',
];

function usePageSettings() {
return useSelect( ( select ) => {
const { canUser, getEntityRecord } = select( coreStore );

const siteSettings = canUser( 'read', 'settings' )
? getEntityRecord( 'root', 'site' )
: undefined;

return {
pageOnFront: siteSettings?.page_on_front,
pageForPosts: siteSettings?.page_for_posts,
};
}, [] );
}

/**
* React hook used to compute the block editor settings to use for the post editor.
*
Expand All @@ -93,8 +108,6 @@ function useBlockEditorSettings( settings, postType, postId ) {
hasUploadPermissions,
canUseUnfilteredHTML,
userCanCreatePages,
pageOnFront,
pageForPosts,
userPatternCategories,
restBlockPatterns,
restBlockPatternCategories,
Expand All @@ -104,17 +117,12 @@ function useBlockEditorSettings( settings, postType, postId ) {
const {
canUser,
getRawEntityRecord,
getEntityRecord,
getUserPatternCategories,
getEntityRecords,
getBlockPatterns,
getBlockPatternCategories,
} = select( coreStore );

const siteSettings = canUser( 'read', 'settings' )
? getEntityRecord( 'root', 'site' )
: undefined;

return {
canUseUnfilteredHTML: getRawEntityRecord(
'postType',
Expand All @@ -128,8 +136,6 @@ function useBlockEditorSettings( settings, postType, postId ) {
: EMPTY_BLOCKS_LIST, // Reusable blocks are fetched in the native version of this hook.
hasUploadPermissions: canUser( 'create', 'media' ) ?? true,
userCanCreatePages: canUser( 'create', 'pages' ),
pageOnFront: siteSettings?.page_on_front,
pageForPosts: siteSettings?.page_for_posts,
userPatternCategories: getUserPatternCategories(),
restBlockPatterns: getBlockPatterns(),
restBlockPatternCategories: getBlockPatternCategories(),
Expand Down Expand Up @@ -229,8 +235,7 @@ function useBlockEditorSettings( settings, postType, postId ) {
// Check these two properties: they were not present in the site editor.
__experimentalCreatePageEntity: createPageEntity,
__experimentalUserCanCreatePages: userCanCreatePages,
pageOnFront,
pageForPosts,
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 think these settings were meant to be experimental. I think we should be ok with removing it, thinks will just keep working fine with undefined.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you are right 🤞

__experimentalUsePageSettings: usePageSettings,
__experimentalPreferPatternsOnRoot: postType === 'wp_template',
templateLock:
postType === 'wp_navigation' ? 'insert' : settings.templateLock,
Expand All @@ -251,8 +256,6 @@ function useBlockEditorSettings( settings, postType, postId ) {
undo,
createPageEntity,
userCanCreatePages,
pageOnFront,
pageForPosts,
postType,
setIsInserterOpened,
]
Expand Down