-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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.
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
Display settings 'label' defined by the 'register_setting' method #59243
Conversation
This pull request changed or added PHP files in previous commits, but none have been detected in the latest commit. Thank you! ❤️ |
Flaky tests detected in 79a3792. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7992936601
|
fd49bec
to
2401c75
Compare
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 Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @itsmewatermelon. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. 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. |
Size Change: +28 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
packages/edit-site/src/components/global-styles/font-library-modal/context.js
Outdated
Show resolved
Hide resolved
fa7353a
to
a44a9bd
Compare
Something to consider here as well is "post type" labels. Like for the default fields but also custom fields. Whatever we come up with here in |
@youknowriad, just to clarify. Do you mean fields registered via The post-type/taxonomy object labels are already available via their selectors - |
@Mamaduka yes. I see these as properties of the "post" or "page" or whatever post type entity, and the settings seem like properties of the "site" entity If I understand properly the purpose of this PR. From core data perspective, we're adding labels to properties of entities. |
That's definitely doable. We can update Currently, entity save flow considers posts/pages and their properties (title, excerpt, meta, etc.) as a single entity, so I'm not sure how valuable the label addition would be without a design update proposal. |
@Mamaduka I'm not suggesting that we do that here. I'm just saying that it's at the same level on core-data so we just need that the decisions made today allow us to do this in the future. It seems that it's the case already :) |
eee7c34
to
fb77e6b
Compare
I'm going to land this. I'm curious how the extra request for the |
👋 This commit seemed to cause the site editor to hang while loading across all browsers. There aren't any errors in the console, so I ran a git bisect to find this commit. Then I confirmed that reverting this commit seemed to solve the issue for me. I'm running wordpress-develop trunk (8d0aed455b1790c5d51386f7675d9e8d68e48edf) if that could be causing issues. |
Hey, @ajlende I can't reproduce the issue locally running Gutenberg trunk and WP 6.5 RC3. I also don't see any e2e tests failing on Do you see any error in the PHP log? |
Nope, the PHP error log is empty too. 😞
I'll do a little more digging in the morning then and report back if I find anything. Thanks for giving it a try. 🙂 |
I traced it to this What is that supposed to return? I get a 200 success code with an empty body. |
I opened #60137 so we don't have to discuss this here anymore. |
…rdPress#59243) * Try: Add 'label' argument to the 'register_setting' method * Use title keyword * Fix route override * Use just a filter * Update getOrLoadEntitiesConfig * Temp update selectors * Finalize first iteration * Fix unit test * Feedback * Restore site entity shortcuts * Update 'getOrLoadEntitiesConfig' doc-block Unlinked contributors: itsmewatermelon. Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: ramonjd <ramonopoly@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: adamziel <zieladam@git.wordpress.org> Co-authored-by: richtabor <richtabor@git.wordpress.org>
I think this PR might have had a small but noticeable impact on "first block" metric in the site editor. I wonder if we could "preload" that options endpoint or something. |
I had some suspicions that it might affect loading. We preload that I don't think we should apply this limitation to the |
An I would've thought that when the data store resolver asks |
Thanks for the feedback, @TimothyBJacobs! If the plugin modifies the schema correctly, the editor should receive an updated schema upon loading. While I understand it's technically possible, what would be the point of changing the schema during the session?
The data store does this for |
I've been thinking about optimizing this, and the answer seems to be a new resolve/selector combo. Maybe something like Why
|
@Mamaduka I can see the reasoning but the same could be said for things like "posts" or "taxonomies" that require a REST API anyway to register the entities and feels like there's no way around the REST API in these cases. |
@youknowriad, a few extra requests are okay for now, but it could be problematic if we adopt the current pattern for other entities. I don't mind exploring the alternative, but I will make this a low priority for now. |
Imagine a plugin with modules, and enabling those modules changes what is returned from a REST API. The plugin that I work on does this outside of the editor, but does use |
Thanks for clarifying, @TimothyBJacobs! |
The 'label' will displayed to users when editing core or custom settings via block editors. It avoids hardcoding Settings labels and improves extensibility. Backports WordPress/gutenberg#59243. Props mamaduka, timothyblynjacobs, ellatrix. Fixes #61023. git-svn-id: https://develop.svn.wordpress.org/trunk@58230 602fd350-edb4-49c9-b593-d223f7449a82
The 'label' will displayed to users when editing core or custom settings via block editors. It avoids hardcoding Settings labels and improves extensibility. Backports WordPress/gutenberg#59243. Props mamaduka, timothyblynjacobs, ellatrix. Fixes #61023. Built from https://develop.svn.wordpress.org/trunk@58230 git-svn-id: http://core.svn.wordpress.org/trunk@57693 1a063a9b-81f0-0310-95a4-ce76da25c4cd
The 'label' will displayed to users when editing core or custom settings via block editors. It avoids hardcoding Settings labels and improves extensibility. Backports WordPress/gutenberg#59243. Props mamaduka, timothyblynjacobs, ellatrix. Fixes #61023. Built from https://develop.svn.wordpress.org/trunk@58230 git-svn-id: https://core.svn.wordpress.org/trunk@57693 1a063a9b-81f0-0310-95a4-ce76da25c4cd
What?
Resolves #41637.
Alternative #45918.
PR improves UI/UX when editing or saving custom settings defined by third-party integrations.
Why?
It avoids hardcoding Settings labels map and improves extensibility. See #41637 for more details.
How?
entityConfig.meta.labels
property.Testing Instructions
wp.data.dispatch( 'core' ).editEntityRecord( 'root', 'site', undefined, { foo_my_setting: 'Testing' } );
.Test setting example
Testing Instructions for Keyboard
Same.
Screenshots or screencast