Store DeepL locale in PLL_Language object#1794
Conversation
Screenfeed
left a comment
There was a problem hiding this comment.
Some points we have to discuss.
Also:
After Facebook and DeepL, we're adding another class property for a 3rd party locale. Then maybe we will have to add another one for the next machine translation service we'll add. And maybe another one someday...
So, here's is an idea:
Instead of adding another property, we could treat these locales differently, a bit like PLL_Language::$term_props.
Technically:
- We keep
$w3cas a class property (like we did for$term_id). - We deprecate the property
$facebook(like we did with$term_taxonomy_idand others). - We create a new private property
$service_locales = array()that will contain$w3c,$facebook, and$deepl. - We create a new public method
PLL_Language::get_service_locale( string $service ): ?string. - The method
Languages::to_array()will need an adjustment. - We use this method in Jetpack and WPSeo integrations.
What do you think?
| if ( ! is_array( $description ) || empty( $description['locale'] ) || ! is_string( $description['locale'] ) ) { | ||
| return null; |
There was a problem hiding this comment.
This changes the current behavior. Previously we were still returning a WP_Language object in case of broken language, while now we return null. Is this something that we want?
| $data['is_rtl'] = ! empty( $data['is_rtl'] ) ? 1 : 0; | ||
|
|
||
| $positive_fields = array( 'term_group', 'page_on_front', 'page_for_posts' ); | ||
|
|
||
| foreach ( $positive_fields as $field ) { | ||
| $data[ $field ] = ! empty( $data[ $field ] ) ? absint( $data[ $field ] ) : 0; | ||
| } | ||
|
|
||
| $data['active'] = isset( $data['active'] ) ? (bool) $data['active'] : true; | ||
|
|
||
| if ( array_key_exists( 'fallbacks', $data ) && ! is_array( $data['fallbacks'] ) ) { | ||
| unset( $data['fallbacks'] ); | ||
| } | ||
|
|
There was a problem hiding this comment.
We shouldn't remove these, this method is used by PLL_Language_Factory::get(), which is used by PLL_Languages::get_list() to build the PLL_Language object, based on the transient value.
If the language term's description needs some sanitization (which is done earlier now), then the transient's value too IMHO.
| * @return string[] | ||
| */ | ||
| private function get_languages() { | ||
| private function get_additional_locales( $locale ) { |
There was a problem hiding this comment.
Types are safe here.
| private function get_additional_locales( $locale ) { | |
| private function get_additional_locales( string $locale ): array { |
Two goals for this PR
PLL_Languageproperty for the DeepL locale.-> to match the other W3C and Facebook locales.
-> to allow to simplify
Deepl::get_target_code().pll_additional_localesto allow developers to add W3C, Facebook or DeepL locales which would not be already included in our predefined list. Only these properties are accepted in this filter.Bonus:
PLL_Language_FactoryPHPStan errors are fixed.