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

Mods: generalize manual activation #1147

Merged
merged 7 commits into from
Apr 9, 2022

Conversation

spleen1981
Copy link
Contributor

@spleen1981 spleen1981 commented Apr 3, 2022

With slugs we introduced the manual activation of a module, adding a checkbox in the integration panel (admin).
Extend this feature to any module, so the activation checkbox is displayed for any module.
This checkbox only applies when all the conditions are met to activate the module (parent plugin activated and no incompatible plugin).

herrvigg and others added 2 commits April 3, 2022 22:15
The tab display is independent from its manual activation.
Any module could have extra settings (e.g. ACF).

Make the display dependent on the module state instead.
Add a new `has_settings` field in the module defintions.
Rename `load_modules_enabled` to `load_active_modules`.
@spleen1981 spleen1981 changed the title modules: generalize manual activation Mods: generalize manual activation Apr 3, 2022
@@ -73,6 +75,7 @@ public static function update_manual_enabled_modules() {
if ( $changed ) {
update_option( 'qtranslate_modules', $options_modules );
self::load_active_modules();
// TODO remove this action
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to give opportunity to modules to perform activation/deactivation actions (e.g. flush rewrite rules for slugs module) on manual state change.
I don't see other clean ways than a do_action here...

Copy link
Collaborator

Choose a reason for hiding this comment

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

I know it's needed, but using a hook for something purely internal is a hack. No need to fix this now but it can always be designed differently ;)
The hooks are the main reason why WordPress became so popular, and one of the reasons this framework is such a huge mess.

@herrvigg herrvigg added plugin: others Concerns integration with other plugins core Core functionalities, including the admin section and removed plugin: others Concerns integration with other plugins labels Apr 4, 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.

A few things fixed on the fly. Will merge but there will be other changes to come.

admin/qtx_admin_options_update.php Outdated Show resolved Hide resolved
admin/qtx_admin_options_update.php Outdated Show resolved Hide resolved
admin/qtx_admin_options_update.php Outdated Show resolved Hide resolved
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.

Good extension. There will be a follow-up.

@herrvigg herrvigg merged commit 89e74c6 into qtranslate:master Apr 9, 2022
@spleen1981 spleen1981 deleted the fix_manual_activation branch April 9, 2022 19:05
herrvigg added a commit that referenced this pull request Apr 10, 2022
Fix regression on front side with the new module handler.
This a quickfix, there should not be any deps in that direction.
herrvigg added a commit that referenced this pull request Apr 10, 2022
The change with `array_merge` introduced a regression.
Default values can't be applied because the unchecked options are
not part of the `$_POST` data.
Yet another quickfix, a proper fix would require more changes.
herrvigg added a commit that referenced this pull request Apr 10, 2022
Reorganize code by moving admin updates to admin source files.

Refactor without the recently added `qtx_modules_update` action.
Re-use `qtranslate_save_config` which is almost equivalent,
except the rules will be rewritten in any case when slugs is
activated, regardress of the changes in options or not.

Remove other option change detection, not really needed.
@herrvigg herrvigg added the modules Related to modules, internal to QTX label Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core functionalities, including the admin section modules Related to modules, internal to QTX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants