Data Views: Add action for pages to set site homepage #65426
+377
−10
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
creativecoder
added
[Feature] Site Editor
|
I thought it was interesting how you handled non-published pages but I noticed an issue; if you set a scheduled page as the homepage it works, but the page remains scheduled resulting in a 404 on the frontend. It might make sense to restrict the action to
The Snackbar copy could use some refinement. Maybe just "Homepage updated"? Testing the flow with reading settings set to display latest posts caused the editor to crash: 
Relating to this, I noticed an oversight in the original design; when setting a static homepage and offering the user the option to set a 
|
creativecoder
force-pushed
the
try/site-editor-homepage-settings
branch
from
ab1c828
to
0f3a986
Compare
|
Size Change: +946 B (+0.05%) Total Size: 1.82 MB
ℹ️ View Unchanged
|
|
@creativecoder is this one ready for review? :) |
|
@jameskoster A few things that still need doing
I'm not sure when I'll be able to get back to this, so if anyone wants to pick this up--feel free! |
|
Thanks @creativecoder! I'm going to have a go at picking this up. I'll start by addressing the items you listed above, and then I'll mark as ready for review. |
Yeah, I see your angle. I agree that we need a better mechanism, we cannot augment the item with every piece of global data we need.
That'd work for me as well. We have some actions defined in the edit-site package. |
|
Thanks for the discussion here 🙏
This sounds like a good path forward, I'll work on that next. |
mikachan
reviewed
Nov 15, 2024
| return decodeEntities( item.title.raw ); | ||
| } | ||
| return ''; | ||
| } |
There was a problem hiding this comment.
I needed to duplicate the getItemTitle function to avoid importing it from the fields package into edit-site.
Yes, I think this could be handled soon after in a follow-up PR. There's also a separate issue for it here: #63667.
I think this is because the action is being called on a page that isn't being previewed. Should selecting an action on a page that isn't the selected/previewed page cause the preview to update to the item the action has been called on? I think I've addressed most of the feedback here, thank you again! After moving the action to edit-site, I'm not sure how best to add the action to the post editor in the post-actions component. I'm not able to import the new action as it lives in a separate package. Would the action need to be duplicated? Edit: It looks like the action only needs to exist in the post editor (done in 7a2960a), so I've removed it from edit-site (it was duplicated otherwise). |
oandregal
reviewed
Nov 18, 2024
| @@ -20,7 +20,7 @@ const TitleView = ( { item }: { item: BasePost } ) => { | |||
| const siteSettings: Settings | undefined = getEntityRecord( | |||
| 'root', | |||
| 'site', | |||
| '' | |||
There was a problem hiding this comment.
For reference, this is fixing #66940 (comment)
Ideally, this parameter should be optional. I think maybe we can make the key: EntityRecordKey above optional?
There was a problem hiding this comment.
Ah yes, I think I've handled that now, in 2b37368.
There was a problem hiding this comment.
Nice, so does that mean we can omit the undefined here?
youknowriad
reviewed
Nov 18, 2024
| }; | ||
| } ); | ||
| const [ currentHomePageTitle, setCurrentHomePageTitle ] = useState( null ); |
There was a problem hiding this comment.
Not sure I understand entirely why this local state is needed? Also, wanted to share that we now have a getHomePage selector that returns { postType, postId } of the current home page. If a static page is set postType will be page otherwise it will be wp_template
oandregal
reviewed
Nov 18, 2024
Comment on lines
+228
to
+237
| // Don't show the action if the user can't manage site options. | ||
| if ( ! canManageOptions ) { | ||
| return false; | ||
| } | ||
|
|
||
| // A front-page template overrides homepage settings, | ||
| // so don't show the setAsHomepage action if it's present. | ||
| if ( hasFrontPageTemplate ) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
These two checks don't use the item. Instead of running them for every item, we can check these when registering the action.
oandregal
reviewed
Nov 18, 2024
| @@ -321,7 +321,7 @@ export interface GetEntityRecord { | |||
| >( | |||
| kind: string, | |||
| name: string, | |||
| key: EntityRecordKey, | |||
| key: EntityRecordKey | undefined, | |||
There was a problem hiding this comment.
This seems to work, and I'm not a TypeScript expert so others may want to chime in. However, I'd expect the GetEntityRecord interface to declare the key as optional as well (as opposed to allow undefined as a value).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Adds an action to set the home page from the site editor.
Fixes #63666
Why?
Currently there is no way to change the home setting (Settings > Reading) from the site editor. This PR adds a way to do that.
How?
Adds a dataview action for pages to set the page as the site homepage that can be triggered from the pages data view.
Testing Instructions
Screenshots or screencast