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

Add support for internationalization. #209

Open
2 tasks
johnhooks opened this issue Apr 2, 2023 · 6 comments
Open
2 tasks

Add support for internationalization. #209

johnhooks opened this issue Apr 2, 2023 · 6 comments
Labels
i18n Internationalisation php Pull requests that update Php code [Scope] Service The core logic of the WP Notify project.

Comments

@johnhooks
Copy link
Collaborator

johnhooks commented Apr 2, 2023

What problem does this address?

There currently isn't any support for translating the notification messages.

What is your proposed solution?

Add a basic translation file to the project.

Requirements

  • generate a POT file.

    Currently running the cli command results in an error.

    wp i18n make-pot ./includes languages/wp-features-notifications.pot
    CLI error output
    PHP Fatal error:  Uncaught TypeError: implode(): Argument #2 ($array) must be of type ?array, string given in     phar:///Users/johnhooks/.bin/wp/php/WP_CLI/DocParser.php:75
    Stack trace:
    #0 phar:///Users/johnhooks/.bin/wp/php/WP_CLI/DocParser.php(75): implode(Array, '\n')
    #1 phar:///Users/johnhooks/.bin/wp/php/WP_CLI/Dispatcher/CompositeCommand.php(32):     WP_CLI\DocParser->get_longdesc()
    #2 phar:///Users/johnhooks/.bin/wp/php/WP_CLI/Dispatcher/CommandFactory.php(113):     WP_CLI\Dispatcher\CompositeCommand->__construct(Object(WP_CLI\Dispatcher\RootCommand), 'cli', Object    (WP_CLI\DocParser))
    #3 phar:///Users/johnhooks/.bin/wp/php/WP_CLI/Dispatcher/CommandFactory.php(45):     WP_CLI\Dispatcher\CommandFactory::create_composite_command(Object(WP_CLI\Dispatcher\RootCommand), 'cli',     'CLI_Command')
    #4 phar:///Users/johnhooks/.bin/wp/php/class-wp-cli.php(481): WP_CLI\Dispatcher\CommandFactory::create('cli',     'CLI_Command', Object(WP_CLI\Dispatcher\RootCommand))
    #5 phar:///Users/johnhooks/.bin/wp/php/commands/cli.php(3): WP_CLI::add_command('cli', 'CLI_Command')
    #6 phar:///Users/johnhooks/.bin/wp/php/WP_CLI/Bootstrap/RegisterFrameworkCommands.php(32): include_once    ('phar:///Users/j...')
    #7 phar:///Users/johnhooks/.bin/wp/php/bootstrap.php(75): WP_CLI\Bootstrap\RegisterFrameworkCommands->process    (Object(WP_CLI\Bootstrap\BootstrapState))
    #8 phar:///Users/johnhooks/.bin/wp/php/wp-cli.php(23): WP_CLI\bootstrap()
    #9 phar:///Users/johnhooks/.bin/wp/php/boot-phar.php(8): include('phar:///Users/j...')
    #10 /Users/johnhooks/.bin/wp(4): include('phar:///Users/j...')
    #11 {main}
      thrown in phar:///Users/johnhooks/.bin/wp/php/WP_CLI/DocParser.php on line 75
    Fatal error: Uncaught TypeError: implode(): Argument #2 ($array) must be of type ?array, string given in     phar:///Users/johnhooks/.bin/wp/php/WP_CLI/DocParser.php:75
    Stack trace:
    #0 phar:///Users/johnhooks/.bin/wp/php/WP_CLI/DocParser.php(75): implode(Array, '\n')
    #1 phar:///Users/johnhooks/.bin/wp/php/WP_CLI/Dispatcher/CompositeCommand.php(32):     WP_CLI\DocParser->get_longdesc()
    #2 phar:///Users/johnhooks/.bin/wp/php/WP_CLI/Dispatcher/CommandFactory.php(113):     WP_CLI\Dispatcher\CompositeCommand->__construct(Object(WP_CLI\Dispatcher\RootCommand), 'cli', Object    (WP_CLI\DocParser))
    #3 phar:///Users/johnhooks/.bin/wp/php/WP_CLI/Dispatcher/CommandFactory.php(45):     WP_CLI\Dispatcher\CommandFactory::create_composite_command(Object(WP_CLI\Dispatcher\RootCommand), 'cli',     'CLI_Command')
    #4 phar:///Users/johnhooks/.bin/wp/php/class-wp-cli.php(481): WP_CLI\Dispatcher\CommandFactory::create('cli',     'CLI_Command', Object(WP_CLI\Dispatcher\RootCommand))
    #5 phar:///Users/johnhooks/.bin/wp/php/commands/cli.php(3): WP_CLI::add_command('cli', 'CLI_Command')
    #6 phar:///Users/johnhooks/.bin/wp/php/WP_CLI/Bootstrap/RegisterFrameworkCommands.php(32): include_once    ('phar:///Users/j...')
    #7 phar:///Users/johnhooks/.bin/wp/php/bootstrap.php(75): WP_CLI\Bootstrap\RegisterFrameworkCommands->process    (Object(WP_CLI\Bootstrap\BootstrapState))
    #8 phar:///Users/johnhooks/.bin/wp/php/wp-cli.php(23): WP_CLI\bootstrap()
    #9 phar:///Users/johnhooks/.bin/wp/php/boot-phar.php(8): include('phar:///Users/j...')
    #10 /Users/johnhooks/.bin/wp(4): include('phar:///Users/j...')
    #11 {main}
      thrown in phar:///Users/johnhooks/.bin/wp/php/WP_CLI/DocParser.php on line 75
  • add translations for an initial set of messages.

@johnhooks
Copy link
Collaborator Author

johnhooks commented Apr 2, 2023

@erikyo I don't have any experience with WordPress localization, do you know the basics so we can get started? Because as soon as we get the db test data filled we will want this in place so we can actually render notifications.

@johnhooks johnhooks added [Scope] Service The core logic of the WP Notify project. i18n Internationalisation php Pull requests that update Php code labels Apr 2, 2023
@erikyo
Copy link
Collaborator

erikyo commented Apr 2, 2023

This is one of the many nice things WordPress does for you, and you don't need anything else to apart adding the "languages" folder in your plugin and do this
https://codex.wordpress.org/I18n_for_WordPress_Developers#Loading_a_Text_Domain

@johnhooks
Copy link
Collaborator Author

johnhooks commented Apr 2, 2023

@erikyo because of how we decided to do the notificaton messages with title_key and message_key, and channels have name_key and description_key, we need to manually add entries in order for them to be looked up by the translating functions. Does that make sense?

@johnhooks
Copy link
Collaborator Author

Summary of thoughts from Discussion #113.

I believe the translation of a notification has to happen when it is emitted and stored in the database for the following reasons:

  • If the code that registers a translation key is removed (either because the plugin stops supporting it or the plugin is uninstalled) the data in the database is broken and the notification can no longer be rendered.
  • Translation is a costly file system based operation. Storing a notification message translated, drastically reduces the number of times a message has to be translated. It would be done once, not on every query for the message.

@erikyo
Copy link
Collaborator

erikyo commented Apr 4, 2023

yes i agree 100%. Currently it seems the most viable and functional option.

If notifications are stored per user it is assumed that user knows the language he uses, so i don't see anything so disadvantageous in this proposal

@johnhooks
Copy link
Collaborator Author

johnhooks commented Apr 4, 2023

Perhaps this system shouldn’t be trying to tackle translations. It should provide a way for plugins to emit notifications. Leave providing translations to the plugins or new developments into how internationalization works. They are very different problems.

It will be difficult to save actionable and possibly customizable messages in a format which can be dynamically rendered in another language, especially when depending on third party code that would be required to indefinitely remain on the system to render the notification.

We could provide an API to query the languages preferences the users subscribed to a channel, and handle linking the correct translated message to the appropriate users when a notification is emitted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
i18n Internationalisation php Pull requests that update Php code [Scope] Service The core logic of the WP Notify project.
Projects
None yet
Development

No branches or pull requests

2 participants