Skip to content
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

Don't show slug metabox on woo shop_orders and shop_coupon pages #1192

Merged
merged 6 commits into from
Jun 18, 2022

Conversation

erichk4
Copy link
Contributor

@erichk4 erichk4 commented Jun 16, 2022

The slug metaboxes might not be necessary for the woocommerce orders and coupon pages...

erichk4 and others added 3 commits June 16, 2022 17:50
The slug metaboxes might not be note necessary for the woocommerce orders and coupon pages...
@herrvigg herrvigg requested a review from spleen1981 June 17, 2022 22:32
@herrvigg herrvigg added the module: WC Integration with Woo Commerce label Jun 17, 2022
Copy link
Collaborator

@herrvigg herrvigg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.
@spleen1981 please could you review? As you have integrated WC with slugs you could have better feedbacks.

I had to rewrite without the null coalescing operator as this is PHP7.0 and QTX still allows PHP 5.4+. The requirements will be raised soon, but until then the retro-compatibility is needed.

modules/slugs/admin/slugs-admin.php Outdated Show resolved Hide resolved
@spleen1981
Copy link
Contributor

Both post and post_type are not guaranteed to exist in $_GET, so they would need to be tested to avoid warnings.
Also post_type seems not to be the right key to test in this case.
Anyways global $post_type seems a cleaner test here.
I think also that is better to move the logic inside the callback, there is no real advantage doing otherwise and readability is worst IMO.
Also I think is better to have an array of "not applicable types" there, as I suspect others may come out.
The above implemented here edcb9ac

@herrvigg as a side note these post types should not be translatable at all (ref. also #1116), we should check upstream and grey-up unchecked the relevant post types checkboxes in qtranslate advanced settings.

@herrvigg herrvigg merged commit 24c15ae into qtranslate:master Jun 18, 2022
@erichk4 erichk4 deleted the patch-1 branch June 19, 2022 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: WC Integration with Woo Commerce
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants