qtranslate / qtranslate-xt

qTranslate-XT (eXTended) - reviving qTranslate-X multilingual plugin for WordPress. A new community-driven plugin soon. Built-in modules for WooCommerce, ACF, slugs and others.
GNU General Public License v2.0
553 stars 104 forks source link

Added filters for the custom_fields and custom_field_classes options #42

Closed herrvigg closed 6 years ago

herrvigg commented 6 years ago

Issue by mweimerskirch Wednesday Feb 25, 2015 at 10:04 GMT Originally opened as https://github.com/qTranslate-Team/qtranslate-x/pull/42


This allows theme and plugin authors to more easily configure custom fields without having the user manually edit the configuration.


mweimerskirch included the following code: https://github.com/qTranslate-Team/qtranslate-x/pull/42/commits

herrvigg commented 6 years ago

Comment by johnclause Friday Feb 27, 2015 at 06:37 GMT


Hey, Michel, that filter would make total sense, if we continued going by this road, but the current plan is to actually combine those two options with one textarea to input json or xml or whatever text which will supply the configuration, like we currently implemented in a number of plugins, listed, for example, in https://github.com/qTranslate-Team/qtranslate-x/pull/40.

The problems with those options is that they go to all pages, which enable LSB (Language Switching Buttons), but they should really go to just pages they need. A newer technique has been implemented under filter 'qtranslate_load_admin_page_config', which is employed in all the integrating plugins done so far. One can specify a lot more information, than it is possible with already obsolete simple "Custom Fields" options.

Could you take a look and see if you can get implemented what you need following the pattern of the code in other integrating plugins? BTW, which plugin are you trying to integrate with that?

herrvigg commented 6 years ago

Comment by mweimerskirch Friday Feb 27, 2015 at 07:51 GMT


I'm not trying to integrate a plugin, but a customer project with lots of custom post types that have custom metaboxes and custom fields.

IMHO, qtranslate_load_admin_page_config is just too complex for use-cases like that. Imagine for example having a post type "contact" with fields "_contact_name", "_contact_title", "_contact_email" (prefixed with underscore so they don't show up in WordPress's native custom fields list). In this case, only the title would have to be translatable. That's a one-liner with the id filter: add_filter('qtranslate_custom_fields', function($fields){ $fields[] = '_contact_title'; return $fields; }); With the qtranslate_load_admin_page_config filter, it would be 5 or more lines to keep it readable. However, if you really dislike the id filter, an alternative would be to introduce some sort of shortcut for the page_config filter specifically for custom post type where you pass the post type name + an array of the field names and be done with it. However, custom fields that are active for all post types (e.g. I sometimes use a "_subtitle" field) would still require more code than with the id filter.

The classname filter can be dropped though IMHO. So far I only see use cases for the id filter.

herrvigg commented 6 years ago

Comment by mweimerskirch Friday Feb 27, 2015 at 23:10 GMT


I though a bit more about the shortcut function for custom post types I talked about above. This shortcut could also hook directly into get_post_meta, so meta values could transparently be translated.

Imagine a filter like this in a theme with custom post types:

add_filter('qtranslate_add_custom_post_type_fields', function($fields){ $fields['contact'] = array('_contact_title'); return $fields; });

Those ids would then automatically be translatable with qtranslate-x. On top of this, when you call the following, you would automatically get the translated value:

get_post_meta($post->ID, '_contact_title', true);

What do you think?

herrvigg commented 6 years ago

Comment by mweimerskirch Wednesday Apr 01, 2015 at 22:29 GMT


@johnclause Is there a way to get this or something similar merged? An ID-based filter would be exactly what we need when working with custom post types for clients. The much longer plugin-oriented syntax is much too complex for most of our purposes.

herrvigg commented 6 years ago

Comment by johnclause Thursday Apr 02, 2015 at 04:53 GMT


Michel, If you do not mind for a temporary solution, just PR what you need (since this one got out of sync), and expect it to be changed in the next few weeks. Would this be all right?

herrvigg commented 6 years ago

Comment by mweimerskirch Thursday Apr 02, 2015 at 06:50 GMT


I rebased this PR so it can be merged automatically again. Will the new solution work in a similar way? (a simple filter where you can add class names, IDs (or more generic jQuery patterns?) so we could upgrade easily) If yes, then I'd appreciate a merge.

herrvigg commented 6 years ago

Comment by johnclause Thursday Apr 02, 2015 at 07:23 GMT


We can design it together to satisfy all the needs. jQuery patterns may be the best. We can also keep multiple ways of doing the things. I will discuss the details with you when time comes, if you do not mind.

herrvigg commented 6 years ago

Comment by johnclause Friday Apr 03, 2015 at 03:52 GMT


Michel, I stored your readme addition for the later use, since we should not advertise what is not final. Instead I added the following one, which should not change anymore:

I am a developer, how can I alter global configuration of qTranslate-X for the specific purpose of my plugin? You can use add_action for hooks qtranslate_loadConfig and qtranslate_admin_loadConfig to alter the configuration stored in global variable $q_config.

Fields 'custom_field' and 'custom_field_classes' are very limiting actually. It would make more sense to have arrays of jquery selectors for content and display hooks. Do you wish to add appropriate functions to common.js? Then you could use filters like 'qtranslate_display_hooks' 'qtranslate_content_hooks' to set such a list of appropriate queries. Or even better, put it in one hook 'qtranslate_field_hooks', with an array argument with two sub-arrays, ['content'] and ['display'], so that one would need to run just one filter for all. We can also eventually parse the current and the future page configurations into such pair of arrays with jquery selectors. Such a construction with jquery selectors may stay permanently in the future, since it seems to be general enough and all other ways can be reduced to this one. What do you think?

herrvigg commented 6 years ago

Comment by mweimerskirch Friday Apr 03, 2015 at 13:04 GMT


I like the idea of using jQuery selectors. It should also be easy to add en "upgrade" option for those that have already entered custom fields on the configuration page.

I'm not sure if I understood the idea of the "content" and "display" filters. Could you elaborate on that? (or maybe show an example on how you would use them).

As to the hooks, I think it is essential to have a dedicated filter specifically to add selectors in order to keep that as simple as possible and encourage plugin and theme creators to use it and integrate with qTranslate-X. The hooks used to modify the configuration are good to have for more specific purposes, but they are also much more complex (e.g. you have to look up which element of the array to modify).

herrvigg commented 6 years ago

Comment by johnclause Friday Apr 03, 2015 at 15:49 GMT


If you look at 'common.js', there are two kind of methods to add a hook, 'addContentHook' and 'addDisplayHook'. In the current page configuration in 'woocommerce-qtranslate-x/qwc-admin.php/qwc-admin.php', for example, we define which kind of hook to add with the attribute 'encode'.

Content hook works on 'input' fields, where users enter some text and LSB switch the text to be entered. Display hook works on non-input fields and switch the text for display purpose only. Functions 'addContentHook' need to know form to which content fields belong to, while 'addDisplayHook' does not. Any more or less advanced plugin has both type of fields to be customized. Woocommerce is an excellent example of it. Since they are both needed most of the time, I thought it would make sense to have one only filter for both, to speed up the things a little bit. Actually, within the same filter we would do all other configuration too, like 'anchors', 'forms', etc.

The currently employed hook 'qtranslate_load_admin_page_config' does it all in one and for all pages too. So, why do we actually need an additional hook? I thought that the advantage of your new hook (let us say it will be named 'qtranslate_admin_page_config') would be that it works for current page, and particular choices for that page are determined inside the hook function called, while 'qtranslate_load_admin_page_config' defines configuration for all possible pages.

It seems to me that it would be easier to read configuration for all possible pages from an xml-like file, which developers would need to setup and maintain. The additional hook via 'qtranslate_admin_page_config' may be useful, if configuration on a page needs to be altered dynamically depending on custom conditions. BTW, the author of Advanced Custom Fields, for example, wanted to have function 'removeContentHook' in 'common.js' to be able to dynamically change the hooks with java script as well.

Does this make sense to you? How would you design it? Do you wish to add methods 'addContentHooksByJQuery' and 'addDisplayHooksByJQuery' in 'common.js', for example? I guess, we will need them in any case. It is easy to follow any example 'addContentHooksBy' or 'addDisplayHooksBy'. I will then make a call of them in an appropriate place.