Post fields: extract title from edit-site to fields package
#66940
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
oandregal
added
[Type] Enhancement
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
oandregal
requested review from
mcsf,
youknowriad,
louwie17,
gigitux and
ntsekouras
oandregal
commented
Nov 12, 2024
Comment on lines
20
to
25
| const siteSettings: Settings | undefined = getEntityRecord( | ||
| 'root', | ||
| 'site', | ||
| '', | ||
| undefined | ||
| ); |
There was a problem hiding this comment.
This was const siteSettings = getEntityRecord( 'root', 'site' ); in the JavaScript version.
There was a problem hiding this comment.
I think for third-argument it's actually clearer because getEntityRecord requires an id in general. But I believe we can make the fourth argument optional =something in the type definition to avoid having to pass it.
youknowriad
reviewed
Nov 12, 2024
| @@ -9,13 +9,16 @@ import { __ } from '@wordpress/i18n'; | |||
| */ | |||
| import type { BasePost } from '../../types'; | |||
| import { getItemTitle } from '../../actions/utils'; | |||
| import TitleView from './title-view'; | |||
|
|
|||
| const titleField: Field< BasePost > = { | |||
| type: 'text', | |||
| id: 'title', | |||
| label: __( 'Title' ), | |||
| placeholder: __( 'No title' ), | |||
| getValue: ( { item } ) => getItemTitle( item ), | |||
There was a problem hiding this comment.
It's nice that this basically means we're reusing the same field for DataForm and DataViews.
There was a problem hiding this comment.
And actions! For the record, I tested the "Duplicate" action in DataViews, DataForm, and Document Inspector.
youknowriad
approved these changes
Nov 12, 2024
|
Size Change: +238 B (+0.01%) Total Size: 1.82 MB
ℹ️ View Unchanged
|
oandregal
force-pushed
the
move/title-field
branch
from
4348bd5
to
f3ac637
Compare
youknowriad
reviewed
Nov 12, 2024
| statusField, | ||
| ======= | ||
| titleField, | ||
| >>>>>>> 4348bd5bbb (Extract title to fields package) |
There was a problem hiding this comment.
I didn't anticipate having to rebase the PRs after one has been merged — I thought git would be smarter :/
oandregal
force-pushed
the
move/title-field
branch
from
f3ac637
to
cda6087
Compare
|
Flaky tests detected in fec23a7. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/11813599705
|
mikachan
reviewed
Nov 15, 2024
| const siteSettings: Settings | undefined = getEntityRecord( | ||
| 'root', | ||
| 'site', | ||
| '' |
There was a problem hiding this comment.
It looks like siteSettings is undefined here, I think it's because of this empty key. If I remove the key parameter altogether, then siteSettings works (i.e. calling getEntityRecord with just root and site).
(Found because I'm working on #65426 😄)
There was a problem hiding this comment.
I just realised this works because the above PR makes key optional in getEntityRecord. Maybe this isn't ideal, although I'm not sure the best way around it.
There was a problem hiding this comment.
Oh, I see. I had to add the empty string for TypeScript not to protest, it wasn't there before. Thanks for providing a fix :)
priethor
added
[Type] Code Quality
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.
Part of #61084
What?
This PR moves the
titlefield definition fromedit-siteto thefieldspackage.Why?
In preparation for moving the whole
usePostFieldshook (conversation).Testing Instructions
Visit the Pages page and verify that the "Title" field still works as expected (search, visualization, edit).
