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

Replace an empty string translation with a space #11

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Traubert
Copy link

@Traubert Traubert commented Nov 26, 2024

In the English UI of Korp, if you do a trend diagram and some part of the corpus is missing time data (like with Ylilauta), the UI prints eg. "non_time_before99.81% of selected material lacks time data."

I now manually fixed this with a whitespace instead of an empty string to reflect the contents of this commit, but this is not the only empty string, so possibly others should be changed too.

In the English UI of Korp, if you do a trend diagram and some
part of the corpus is missing time data (like with Ylilauta), the UI
prints eg. "non_time_before99.81% of selected material lacks time data."

I now manually fixed this with a whitespace instead of an empty string
to reflect the contents if this commit, but this is not the only empty
string, so possibly others should be changed too.
Copy link

@janiemi janiemi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Traubert, thanks for reporting and fixing the issue. I think the root cause is that the localization code handling translations replaces empty values with the translation key, in effect treating them as missing translations. Returning the translation key for a completely missing translation is a deliberate fall-back.

Språkbanken’s Korp also seems to be affected by the same issue: see the screenshot
from this search:

Screenshot_Korp_SBX_trend_diagram_non_time_before_Eng_20241127

I didn’t find any related GitHub issue, so I think we should report it. Would you do that, too, or would you prefer me to do? As far as I can see, the localization code in Språkbanken’s jquery.localize.js is still legacy code using JQuery and will probably be rewritten in the course of migrating the Korp frontend to Vue. (I’ve made a small change to the code in question in our Korp.)

I think that we could fix the issue now in our Korp by changing the empty translation. However, I also tried fixing the root cause in commit a4e2b27a on branch fix-empty-translation. (I don’t know if and how I could have proposed the change to a completely different file in a comment of this pull request, so I created a new branch but not a pull request.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants