-
Notifications
You must be signed in to change notification settings - Fork 897
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
Decouple primary term second round #21361
base: feature/decouple-hidden-fields
Are you sure you want to change the base?
Decouple primary term second round #21361
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 🏗️
Please add the why of things in your PR.
The base is pointing towards your other PR, I think you would want to merge into the feature branch?
f0a950e
to
ee04d6c
Compare
59567de
to
c8dde6c
Compare
Co-authored-by: Igor <35524806+igorschoester@users.noreply.github.com>
Co-authored-by: Igor <35524806+igorschoester@users.noreply.github.com>
e0ddefa
to
20e8eeb
Compare
Pull Request Test Coverage Report for Build 04df54d03c4cae059a7cff413a64420a715f62b4Details
💛 - Coveralls |
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 ✅
AT 🚫
Without the metabox, it does not save anymore
eslint fix cs
e0d36f6
to
6fe438c
Compare
I'm putting this PR n hold since we might want to address that from a different perspective: |
Context
show_in_rest
as positive when registering the meta. That is done in the wpseo meta class. However, the primary terms meta is not added to the hidden fields from the$meta_fields
. I added the primary terms in the init function of the wpseo meta class. After further examination we saw that registering the meta fields is done in a specific time and we want to be more flexable for taxonomies that are being registered in a later stage.Summary
This PR can be summarized in the following changelog entry:
Relevant technical choices:
rest_api_init
and we are not limited to the WPSEO meta init method.Test instructions
Test instructions for the acceptance test before the PR gets merged
This PR can be acceptance tested by following these steps:
Inspect the the page and look for an input with the id
yoast_wpseo_primary_category
. Manually change it's value to text, zero, or a negative number and save.Refresh the page and inspect the page.
Without the metabox
class-metabox.php
:// add_action( 'add_meta_boxes', [ $this, 'add_meta_box' ] );
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 #21223