Add fallback i18n if strings are missing from locale #26

denisebitca · 2025-04-10T21:09:11+02:00

denisebitca commented

2025-04-10 21:09:11 +02:00

On page load, at most three locales are loaded: user locale
from Accept-Language, the default locale (if they aren't the
same), and the template locale. Using function getIntlString(),
if the user locale doesn't have a string, the string is fetched
from the default locale (if they aren't the same), and then,
finally, if neither the user or the default locale have the string,
it is taken from the template (which only contains string labels).

On page load, at most three locales are loaded: user locale from Accept-Language, the default locale (if they aren't the same), and the template locale. Using function getIntlString(), if the user locale doesn't have a string, the string is fetched from the default locale (if they aren't the same), and then, finally, if neither the user or the default locale have the string, it is taken from the template (which only contains string labels).

👍 1

denisebitca added 1 commit 2025-04-10 21:09:11 +02:00

Add fallback i18n if strings are missing from locale f443342a78

On page load, at most three locales are loaded: user locale
from Accept-Language, the default locale (if they aren't the
same), and the template locale. Using function getIntlString(),
if the user locale doesn't have a string, the string is fetched
from the default locale (if they aren't the same), and then,
finally, if neither the user or the default locale have the string,
it is taken from the template (which only contains string labels).

miraty commented

2025-04-10 22:04:57 +02:00

Have you considered setting appropriate lang attributes? I think that would help screen readers in case of mixed language content.

Have you considered setting appropriate `lang` attributes? I think that would help screen readers in case of mixed language content.

denisebitca commented

2025-04-10 22:24:37 +02:00

@miraty wrote in #26 (comment):

Have you considered setting appropriate lang attributes? I think that would help screen readers in case of mixed language content.

We could add a child tag in case of mixed language content. Something like data maybe?

Replacing

goto a;

with something like

return "<data lang=\"" . DEFAULT_LOCALE . "\">" . $return_loc[$string_label] . "</data>";

...and so on and so forth, if necessary.

But could this idea be tested with a screen reader?

@miraty wrote in https://code.antopie.org/miraty/libreqr/pulls/26#issuecomment-272: > Have you considered setting appropriate `lang` attributes? I think that would help screen readers in case of mixed language content. We could add a child tag in case of mixed language content. Something like [data](https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/data) maybe? Replacing https://code.antopie.org/miraty/libreqr/src/commit/f443342a78dd9256d24de9ea0613e4aca29d3679/index.php#L59 with something like ```php return "<data lang=\"" . DEFAULT_LOCALE . "\">" . $return_loc[$string_label] . "</data>"; ``` ...and so on and so forth, if necessary. But could this idea be tested with a screen reader?

denisebitca added 1 commit 2025-04-10 22:33:51 +02:00

Added <data lang=[locale]> tag for mixed i18n fallback content 4e7b8bead0

In order to make content more accessible for screen reader users,
the <data> tag with the lang attribute is now inserted if i18n
strings have to fallback to another locale.

denisebitca commented

2025-04-10 22:35:28 +02:00

4e7b8bead0 doesn't break the CSS when it has to fallback. Requires screen reader testing, however.

https://code.antopie.org/miraty/libreqr/commit/4e7b8bead0a64a33fcd51bd8a5b6d13b116831ea doesn't break the CSS when it has to fallback. Requires screen reader testing, however.

miraty commented

2025-04-10 22:45:45 +02:00

The value attribute is required for the data element according to the HTML Standard. I think you should just use a span element instead.

But could this idea be tested with a screen reader?

Requires screen reader testing

I never tested anything with a screen reader, I just know that appropriately tagging the language is generally considered a good practice.

The `value` attribute is required for the `data` element according to the [HTML Standard](https://html.spec.whatwg.org/multipage/text-level-semantics.html#the-data-element). I think you should just use a `span` element instead. > But could this idea be tested with a screen reader? > Requires screen reader testing I never tested anything with a screen reader, I just know that appropriately tagging the language is generally considered a good practice.

denisebitca added 1 commit 2025-04-10 22:58:46 +02:00

Swapped data with span on i18n fallback, fixed placeholder bug a63bff9ea7

Textarea placeholders are attributes and not inner HTML content.
Therefore, we need to take them into account and adapt the string.
Instead, we set the textarea to the fallback locale via the lang
attribute.

Furthermore, data requires the value attribute, whereas span does
not, and span does not break the CSS.

denisebitca commented

2025-04-10 22:58:58 +02:00

@miraty wrote in #26 (comment):

The value attribute is required for the data element according to the HTML Standard. I think you should just use a span element instead.

But could this idea be tested with a screen reader?

Requires screen reader testing

I never tested anything with a screen reader, I just know that appropriately tagging the language is generally considered a good practise.

Okay, let's try using a span then.

@miraty wrote in https://code.antopie.org/miraty/libreqr/pulls/26#issuecomment-276: > The `value` attribute is required for the `data` element according to the [HTML Standard](https://html.spec.whatwg.org/multipage/text-level-semantics.html#the-data-element). I think you should just use a `span` element instead. > > > But could this idea be tested with a screen reader? > > > Requires screen reader testing > > I never tested anything with a screen reader, I just know that appropriately tagging the language is generally considered a good practise. Okay, let's try using a span then.

miraty commented

2025-04-10 23:06:15 +02:00

getIntlString returns HTML content but is sometimes used inside an HTML attribute, which is invalid. You should give up language taging in such cases, unless you find another solution.

`getIntlString` returns HTML content but is sometimes used inside an HTML attribute, which is invalid. You should give up language taging in such cases, unless you find another solution.

miraty commented

2025-04-10 23:15:22 +02:00

I didn't notice your placeholder hacks. Can you try using something like a boolean function argument to disable language taging instead? As I said on my previous message, there are other places where that will be a problem.

denisebitca commented

2025-04-10 23:15:45 +02:00

@miraty wrote in #26 (comment):

getIntlString returns HTML content but is sometimes used inside an HTML attribute, which is invalid. You should give up language taging in such cases, unless you find another solution.

Sure, I'll add a parameter to set the format

@miraty wrote in https://code.antopie.org/miraty/libreqr/pulls/26#issuecomment-279: > `getIntlString` returns HTML content but is sometimes used inside an HTML attribute, which is invalid. You should give up language taging in such cases, unless you find another solution. Sure, I'll add a parameter to set the format

👍 1

denisebitca commented

2025-04-10 23:18:31 +02:00

@miraty wrote in #26 (comment):

I didn't notice your placeholder hacks. Can you try using something a boolean function argument to disable language taging instead? As I said on my previous message, there are other places where that will be a problem.

I'm thinking of doing an argument with three possible values: html, attribute and raw.

@miraty wrote in https://code.antopie.org/miraty/libreqr/pulls/26#issuecomment-280: > I didn't notice your placeholder hacks. Can you try using something a boolean function argument to disable language taging instead? As I said on my previous message, there are other places where that will be a problem. I'm thinking of doing an argument with three possible values: html, attribute and raw.

miraty commented

2025-04-10 23:19:54 +02:00

What is the difference between attribute and raw?

denisebitca commented

2025-04-10 23:22:16 +02:00

@miraty wrote in #26 (comment):

What is the difference between attribute and raw?

I was going to say to add the lang attribute in placeholders, but I just checked, and placeholders are a visual element only. Nevermind, boolean it is.

@miraty wrote in https://code.antopie.org/miraty/libreqr/pulls/26#issuecomment-283: > What is the difference between attribute and raw? I was going to say to add the lang attribute in placeholders, but I just checked, and [placeholders are a visual element only](https://design.va.gov/components/form/textarea#accessibility-considerations). Nevermind, boolean it is.

denisebitca added 1 commit 2025-04-10 23:30:14 +02:00

Added a "raw" parameter to the getIntlString function b80ec412be

Multiple attribute fields depended on the locale strings, which
made inserting HTML with a lang attribute impossible. Furthermore,
placeholders are purely visual, so their textareas do not need
a lang attribute. It is now possible to obtain a string with no
HTML added to it.

miraty commented

2025-04-11 15:52:41 +02:00

You missed enabling the raw argument on getIntlString('alt_QR_before') and getIntlString('alt_QR_after').

There are unnecessary tabs on line 65 and end of line 68.

if statements on line 55 and 57 can be merged.

Also I think that could help with readability to invert the outer condition at the beginning of getIntlString, like so:

if (array_key_exists($string_label, $chosen_loc))
	return $chosen_loc[$string_label];
// The rest of the function outside a condition

If you don't have reasons to do this, consider using a consistent style for these points:

The opening bracket on a dedicated line
The use of # to comment instead of //

Everything else seems okay!

You missed enabling the `raw` argument on `getIntlString('alt_QR_before')` and `getIntlString('alt_QR_after')`. There are unnecessary tabs on line 65 and end of line 68. `if` statements on line 55 and 57 can be merged. Also I think that could help with readability to invert the outer condition at the beginning of `getIntlString`, like so: ```php if (array_key_exists($string_label, $chosen_loc)) return $chosen_loc[$string_label]; // The rest of the function outside a condition ``` If you don't have reasons to do this, consider using a consistent style for these points: - The opening bracket on a dedicated line - The use of `#` to comment instead of `//` Everything else seems okay!

denisebitca added 1 commit 2025-04-11 16:06:28 +02:00

Clean up code according to code style, fixed missing raw arguments 3490537e67

denisebitca commented

2025-04-11 16:08:01 +02:00

Should be good, you can go ahead and review @miraty :)

miraty requested changes 2025-04-11 16:11:51 +02:00

index.php Outdated

					
				@ -31,0 +46,4 @@

				function getIntlString(

					$string_label,

					$raw = false

				){

miraty commented

2025-04-11 16:09:25 +02:00

Please use a space before the bracket

denisebitca commented

2025-04-11 16:15:34 +02:00

Done

denisebitca marked this conversation as resolved

denisebitca added 1 commit 2025-04-11 16:14:44 +02:00

Fix missing space before opening bracket in function declaration 701cf12d0c

miraty requested changes 2025-04-11 16:17:31 +02:00

index.php

					
				@ -31,0 +35,4 @@

				{

					require "locales/" . DEFAULT_LOCALE . ".php";

					$default_loc = $loc;

				}

miraty commented

2025-04-11 16:17:26 +02:00

Still inconsistent brackets use in this block

denisebitca commented

2025-04-11 16:21:38 +02:00

Done

denisebitca marked this conversation as resolved

denisebitca added 1 commit 2025-04-11 16:20:43 +02:00

Fix inconsistant bracket use ef47b6d4b2

miraty merged commit 80ee070168 into main

2025-04-11 16:28:02 +02:00

miraty referenced this pull request from a commit

2025-04-11 16:28:03 +02:00

Add fallback i18n if strings are missing from locale (#26)

denisebitca commented

2025-04-11 16:28:26 +02:00

LGTM!!!! 🎊

🎉 1

Sign in to join this conversation.

No reviewers

No labels

No milestone

No project

No assignees

2 participants

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: miraty/libreqr#26

Rows
Columns