Hard code time to leave string in window title

[SarahRemus]

Related issue

Closes #912

Context / Background

At the moment, the Time to Leave string is part of the translations. In particular, it concerns these cases: Preferences - title, WorkdayWaiver - title.
This can cause potential errors in the translation when the translator is translating the time-to-leave string as well (which should not be the case). It happened before in issue https://github.com/thamara/time-to-leave/issues/861 where the title was translated in Hindi and Gujarati.

What change is being introduced by this PR?

Instead of setting the title of the Preference and WorkdayWaiver window in the respecting html file, I set the title where the new Browser window is created with prefWindow.setTitle(Time to leave - ${getCurrentTranslation('$Preferences.title')}).
This way, Time to leave can't be translated on accident.

If you would like to use this solution, we have to remove the time-to-leave string from all translation files. I just wanted to wait with this until I know if you want to proceed with this solution.
Without removing the string from the translation file, the time-to-leave title is currently shown twice in the window title. Just wanted to mention that, so you are not confused what's going on.

If the language is changed, the title of the preference window will only update once the window is closed. But I think that is the case for all translations set not directly in the html file?

How will this be tested?

Open the Preference or WorkdayWaiver window. You will see the title Time to leave - Time to leave - Preferences/ Workday Waiver which is the expected behavior at the moment and shows that the first time-to-leave string is set in js.

[codecov]

Codecov Report

Merging #931 (f7b849d) into main (3040f66) will decrease coverage by 0.14%.
The diff coverage is 0.00%.

❗ Current head f7b849d differs from pull request most recent head 077b0c9. Consider uploading reports for the commit 077b0c9 to get more accurate results

@@            Coverage Diff             @@
##             main     #931      +/-   ##
==========================================
- Coverage   70.38%   70.25%   -0.14%     
==========================================
  Files          27       27              
  Lines        2161     2165       +4     
  Branches      323      323              
==========================================
  Hits         1521     1521              
- Misses        640      644       +4     

Impacted Files	Coverage Δ
src/preferences.js	82.41% <0.00%> (-1.86%)	⬇️
src/workday-waiver.js	81.64% <0.00%> (-0.62%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[araujoarthur0]

You could instead use document.title to change the page from within the renderer pages. Then you can also update the preferences page title with the language update.
https://stackoverflow.com/a/6715344

[SarahRemus]

I tried using document.title to change the title.
I'm not sure if I understood you @araujoarthur0 correctly, so I will describe what I did.
Within the workday-waiver.html file, I introduced this script in the header part: <script>document.title = 'Example title'</script>.
But the problem is, that every time you open the Workday Waiver window, for a second you can see Time to Leave as title before it changes to Example title.

Is that what you suggested me to do, or did I miss something?
Can this glitch be fixed somehow?

[araujoarthur0]

@SarahRemus you can add this call to the document ready block in workday waiver.js and preferences.js. In preferences you can add another call to document.title in the callback to language changed.
We do have some visual glitches in regards to Windows loading that we couldn't handle yet, like the time taken for the window to apply the theme and create contents.
Maybe we can solve this by waiting a bit to show the browser window.

[SarahRemus]

I'm having a really hard time implementing your suggestions.
I just can't find a way how to use getCurrentTranslation(..) in preferences.js. And therefore I can't change the title with $(document).attr("title", Time to Leave - ${getCurrentTranslation('$Preferences.title')}); in preferences.js.

On another note, I'm not sure if this is the smart way to go. Typically, a user only changes the language preference once. With the implementation I proposed before (prefWindow.setTitle(Time to Leave - ${getCurrentTranslation('$Preferences.title')})), the preference title would only be updated after the window is closed. BUT this would be the only small drawback. From now on, the window titles are displayed perfectly.

However, if we change the title with $(document).attr("title", Time to leave - ${getCurrentTranslation('$Preferences.title')}); in preferences.js we would have a visual glitch every time the window is opened, or we would have to delay the opening of the window to make the glitch invisible.
And I really don't know if that is worth it, only to change the preference window title immediately once the language is changed.

[araujoarthur0]

I'm having a really hard time implementing your suggestions.
I just can't find a way how to use getCurrentTranslation(..) in preferences.js. And therefore I can't change the title with $(document).attr("title", Time to Leave - ${getCurrentTranslation('$Preferences.title')}); in preferences.js.

Hi @SarahRemus, you can check https://github.com/thamara/time-to-leave/blob/3040f66028c1df445b580b61eef80e4244d3daeb/src/preferences.js#L37 to see the language data that is returned by the main process to the renderer process once the language is changed.
At this step, you have languageData, which can be leveraged for the translation.

On another note, I'm not sure if this is the smart way to go. Typically, a user only changes the language preference once. With the implementation I proposed before (prefWindow.setTitle(Time to Leave - ${getCurrentTranslation('$Preferences.title')})), the preference title would only be updated after the window is closed. BUT this would be the only small drawback. From now on, the window titles are displayed perfectly.

However, if we change the title with $(document).attr("title", Time to leave - ${getCurrentTranslation('$Preferences.title')}); in preferences.js we would have a visual glitch every time the window is opened, or we would have to delay the opening of the window to make the glitch invisible.
And I really don't know if that is worth it, only to change the preference window title immediately once the language is changed.

We already have this glitch with the theme loading, so if we add the delay, it'll fix both issues at once. But we can open another issue to add this delay, you don't need to add it here. We also already have today the Preferences title changing as the page loads.
I believe changing the title on the go is a better experience for users as they see all the language transformations at once in the visible window (I would even rather prefer that the main window got translated at the same time, but that's for another day).

[SarahRemus]

Okay now I think I finally get what you @araujoarthur0 mean! :) Sorry it took me so long. I think now everything is working as you suggested.

[araujoarthur0]

I was reviewing the code once more, and I think there is a better place to add the translation code rather than what I suggested before 😥
The method translatePage() is called by the two renderer windows, and already includes the proper usage of the languageData file. My suggestion is to add a windowName parameter to the function and pass it from preferences and workday waiver.
Then you can just call from within translatePage()

const translatedTitle = languageData.data.translation[windowName].title; (there might be a better way to use a variable here)
$(document).attr('title', `Time to Leave - ${translatedTitle}`);

If you do this, you don't have to add the two pieces of code you added to preferences.js, as setupLanguages() already calls this eventually.
After this, feel free to remove the time to leave string from the language files!

[SarahRemus]

Okay, I moved the functionality into translatePage() and removed the Time to Leave String from all the language files.

[araujoarthur0]

Thanks! I'm just not seeing the changes in locales\en\translation.json yet.

[SarahRemus]

I must have overlooked that! Thanks for pointing it out, I included the changes.

[araujoarthur0]

\changelog-update
Message: Fix #912: Hard code time to leave string in window title