-
Notifications
You must be signed in to change notification settings - Fork 894
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
21141 decouple hidden fields for metabox #21161
21141 decouple hidden fields for metabox #21161
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CR 🏗️
Why are you not removing the side-effects in the actions?
If you are creating a sync that listens to the store and sets the inputs. The actions can be freed of that responsibility?
Did you investigate using a REST route instead of the script data for retrieving the initial values?
fefd497
to
a420578
Compare
… hidden and we need to register the metadata.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CR 🏗️
…den-fields-for-metabox
c0a9cb0
to
9e5cbd0
Compare
Meta property is not present in the core and core/editor store when editing a custom post type with block editor. Seems like a bug in WordPress |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AT 🥳
1ed281f
into
feature/decouple-hidden-fields
Context
core/editor
store for update. This is the first step in decoupling the metabox from the block editor and part of the investigation done in PoC for decoupling metabox and sidebarSummary
This PR can be summarized in the following changelog entry:
Relevant technical choices:
core/editor
store.src/integrations/third-party/wordproof.php
and moved the code tosrc/initializers/wordproof-metadata.php
because the hook for adding extra fields needs to be used in an initialiser in order to register the metadata. We need to register the metadata to have it available in thecore/editor
store.wordproof
in the metabox translations method because adding a translation also adds the fields to the hidden fields without any check. The wordproof-metadata class also adds those translations.get_options
in the conditionalsrc/conditionals/third-party/wordproof-integration-active-conditional.php
because WPSEO option class does not work in the initialiser.schema_page_type
ininc/class-wpseo-meta.php
line 422 , all other metadata defs are usingdefault_value
and the property is used in when getting the value.admin/formatter/class-post-metabox-formatter.php
metabox
window object and then used it in the store for initial state of the default values. That is used then in the dropdown list of schema types.render_hidden_fields
methods because the sync will populate those fields on load. We want one source of truth for the metadata, so now those methods only render hidden fields without value.createWatcher
for cleaner and safer import and for testing proposes.createCollectorFromObject
to helpers in editor-modules to be used in syncs in add-ons.onLoad
actions since we are now using the initial state when registering the store to load the values.add_extra_wpseo_meta_fields
that needs to be done in an initializer class to actually register the meta, which was not the case here. I added it to the meta key in the meta class fields and removed it's classes since we are checking there if it's a post. This check is not done for other fields that are for posts only and there is no problem with that. Also when adding the translations intranslate_meta_boxes
it adds those fields without any check, so even though we are checking if it's a post, we are adding the fields anyway through the translations .edit_posts
capabilities. That is why I added thecontext
props inshow_in_rest
schema
property.get_show_in_rest_args
because we don't want to setshow_in_rest
to true when we the fields is a non-form one.yoast_wpseo_metadesc
from theelementTarget
list because it's an id of a hidden field for elements that their change should refresh the analysis. I trigger the refresh in the subscribe method below. That is why I changed the name ofrefreshAfterFocusKeywordChange
to refreshApp because I'm using it also for the description.Metadata_Groups
class to have the logic of the visibility of the metadata fields groups in one place, since it is used both in the metabox classes and the formattersTest instructions
Test instructions for the acceptance test before the PR gets merged
This PR can be acceptance tested by following these steps:
Test instructions and code review divided to issues:
Relevant test scenarios
Test instructions for QA when the code is in the RC
QA can test this PR by following these steps:
Impact check
This PR affects the following parts of the plugin, which may require extra testing:
UI changes
Other environments
[shopify-seo]
, added test instructions for Shopify and attached theShopify
label to this PR.Documentation
Quality assurance
Innovation
innovation
label.Fixes Decouple hidden fields for metabox