jjgrainger / PostTypes

Simple WordPress custom post types.
https://posttypes.jjgrainger.co.uk/
MIT License
373 stars 47 forks source link

t10ns #2

Closed richardsweeney closed 6 years ago

richardsweeney commented 8 years ago

Hi, class looks great and works well, but I haven't managed to get the t10ns working.

I added the this via composer to my theme (if i require it to my vendor dir, the t10ns won't be found). I could translate all the strings ok, but the t10ns don't actually work.

The issue is unfortunately with using a variable for the text-domain for t10ns. Otto sums it up nicely in the following article (it's old but still relevant).

http://ottopress.com/2012/internationalization-youre-probably-doing-it-wrong/

Basically, the get_text function suite doesn't parse PHP. It only searches through the code for __() _e() etc. The textdomain must therefore be a string.

Did you ever get this working with a provided textdomain for the object instance?

<?php
    $case = new PostType('case');
    $case->translation('shipyard');
?>

As i said, I can grab the t10ns via poedit, but the translations are not show in the backend.

jjgrainger commented 8 years ago

@richardsweeney I believe the translation (when passing the textdomain for the object instance) works with a few translations plugins.

But, as for working with poedit, I think the only way it can work with the class is supplying the translation strings in the arguments.

So for example, to set up the post type labels for translation with poedit, you would do the following:

$args = [
  'labels' => [
    'name' => __('Cases', 'shipyard'),
    'singular_name' => __('Case', 'shipyard'),
    'add_new_item' => __('Add New Case', 'shipyard')
    // ... additional labels here
  ]
];

$case = new PostType('case', $args);

I know its additional work on the developers part, but seeing how get_text doesn't parse PHP I don't see any other way around it, unfortunately :(

Thanks for the article 👍 . There are definitely parts of the class that can be fixed following the articles advice.

jorisvm commented 7 years ago

The translation is still an issue. Not the gettext compatibility, but simply getting a valid .mo file to work with this class.

I ran into it, and it comes down execution order. The name and label strings are initialized on class construction. In other words you can call the PostType->translation() method but it is always to late. I made some modifications to the code to make it work by calling PostType->names() and PostType->labels() on init instead of on construct and it does the trick. I suppose I could send it as a pull request if you want me to.

jjgrainger commented 7 years ago

I've been researching into this more while working towards releasing v2.0 and I've decided to remove anything to do with translations from the project.

With the current setup, I seem to be encouraging bad practices around translations such as using a variable for the text domain and more. Likewise, PostTypes isn't a plugin but a developer utility. Therefore, I believe it should not have its own text domain or translate strings itself.

For v2.0, I plan to remove anything to do with translations and instead provide simple methods to set labels and other strings that need translations. This way the developer will have more control and be able to implement translations properly.

jorisvm commented 7 years ago

Thanks for the reply, I understand that using a variable for translations may not be best practice, but it would be real nice if you could somehow tell the class to use the textdomain of the theme or plugin for which the posttype class is used. For plugin or theme development centralizing all translations in 1 po file and one text domain is crucial. Maybe create a way to set all translation defaults from outside the class?

$books = new PostType(array(
            'name'  => 'book',
            'singular' => __('Book', 'my-plugin'),
            'plural' => __('Books', 'my-plugin'),
            'slug' => 'book'
));

$books->setTranslation(array(
            'add_new' => __('Add New', 'my-plugin'),
            'add_new_item' => sprintf(__('Add New %s', 'my-plugin'), $books->singular),
            'edit_item' => sprintf(__('Edit %s', 'my-plugin'), $books->singular),
            'new_item' => sprintf(__('New %s', 'my-plugin'), $books->singular),
            'view_item' => sprintf(__('View %s', 'my-plugin'), $books->singular),
            'search_items' => sprintf(__('Search %s', 'my-plugin', $books->plural),
            'not_found' => sprintf(__('No %s found', 'my-plugin'), $books->plural),
            'not_found_in_trash' => sprintf(__('No %s found in Trash', 'my-plugin'), $books->plural),
            'parent_item_colon' => sprintf(__('Parent %s:', 'my-plugin'), $books->singular)
));

Just a thought. Not exactly fast code to write, but it would prevent using a variable textdomain.

Looking forward how this wil workout in the 2.0 version. Keep up the good work.

jorisvm commented 6 years ago

Nicely solved in 2.0!

jjgrainger commented 6 years ago

@jorisvm Thanks!