lostindark / DriverStoreExplorer

Driver Store Explorer [RAPR]
GNU General Public License v2.0
6.63k stars 389 forks source link

Adding Hebrew locale #136

Closed ItielMaN closed 3 years ago

ItielMaN commented 3 years ago

Hi,

I'm using the tool a lot and recently I noticed you added RTL support (by supporting Arabic, I assume). This motivated me to translate the tool into Hebrew, and I'm almost done.

A few questions/notes though:

  1. For Message_Driver_Added_Error, I think the "the package" should come before the placeholder, like in Message_Driver_Added.
  2. I do not understand Message_Driver_Added_Installed_Error. What does the & contribute to the sentence? (What I think happened is that you copy-pasted the Message_Driver_Added_Installed contents, modified the first part of the sentence and forgot to change "installed" to "installing". Is that the case? If not, I'm lost...)
  3. Is there somewhere I can actually see the entire string of Dialog_Open_Filters (etc) in the tool? I can only see the INF files part in the open file dialog, but not the other part. I assume that the | *.inf part is not visible to the user, and is meant to tell Windows what files to filter?
  4. Do I need to keep the comment tags and contents or should they be removed, as I see in other translation files?
  5. A bit off-topic: Can you add RTL support for the table itself? Currently it's LTR also when on RTL mode (and if you do, please make sure to force LTR (but not left-align) the contents of the table (but not the headers)). Adding support for dialogs (including the About one, but not so important as normal dialogs) could also be nice.
  6. Also off-topic: You can make the Delete Driver(s) disabled when no entry is checked or selected.

Also, I'd like (if possible) to test first the tool with the lang file I'd supply when I'll finish before you publish a new version, as from my experience adding an RTL language is not quite smooth as adding any other language, and I'm not sure if the Arabic localizer has reported issues with the UI with the Arabic translation, if there are even any.

Also also, tell me if you'd like separate issues filed for 5 and 6 above.

Thanks!

lostindark commented 3 years ago
  1. Yes. That's an existing bug.
  2. It should be installing.
  3. Yes, Windows diaglog will only show the first part in the dropdown, and the part on the right of | is just for filter.
  4. You should only keep it if you have language specific comment there.
  5. RTL support was contribute by other dev. If you have knowledge in this, you can contribute the fix.
  6. Please file separate issue.
ItielMaN commented 3 years ago

RTL support was contribute by other dev. If you have knowledge in this, you can contribute the fix.

I see. I know a bit of C# but couldn't find a suitable solution to fix RTL for messageboxes without changing every one of them to also have MessageBoxOptions.RightAlign and MessageBoxOptions.RtlReading. I also couldn't adjust the About dialog to RTL so I stopped there. I'm attaching here the complete lang file for Hebrew just so I don't lose it, but please don't add it to the tool as having mixed RTL&LTR text (like in Message_ForceDelete_Single_Package on LTR layouts) will display the text in the wrong order. So imo it's better to have no translation than to have the text like that.

I'll ask the Arabic translator if they can improve the RTL layout.

Language.he-IL.zip

ItielMaN commented 3 years ago

I'll ask the Arabic translator if they can improve the RTL layout.

Apparently that's not always possible on GitHub.

Hi @courrier1, I see you've added RTL support to the app, thank you for that! See the above conversation, I was wondering if you could submit a PR, if you are willing and have the time, for improving the RTL support for (primarily) the messageboxes and (optional, but appreciated) other dialogs such as the About dialog, the Select Driver Store dialog, the main driver table etc?

lostindark commented 3 years ago

I made some RTL fixes, you can try the bits here: https://ci.appveyor.com/api/buildjobs/rqogxvdgqt3imv6t/artifacts/Rapr%2Fbin%2FDriverStoreExplorer.v0.11.44.zip

ItielMaN commented 3 years ago

I made some RTL fixes, you can try the bits here: https://ci.appveyor.com/api/buildjobs/rqogxvdgqt3imv6t/artifacts/Rapr%2Fbin%2FDriverStoreExplorer.v0.11.44.zip

@lostindark Thank you, this looks so much better!

If and when you have the time, it'd be nice if you could:

  1. Fix the dialog boxes to be RTL
  2. Fix the "Scanning driver store..." to be RTL (currently the 3 dots are on the right side of the text when in RTL, they should be on the left)
  3. Force the size fields under Size to be LTR (if not localized, size units should look in RTL the same as LTR, e.g. "15 MB" and not "MB 15"). Also if possible, right-align the "Size" header when in RTL

The second and third issues here are somewhat minor, but unfortunately the first one is a major one so I still prefer if you would not add my lang file at this time.

lostindark commented 3 years ago

Which dialog boxes are not RTL?

ItielMaN commented 3 years ago

@lostindark I mean the message dialogs. The ones like Message_Delete_Single_Package, Message_Delete_Multiple_Packages, Message_Sure etc.

lostindark commented 3 years ago

Fixed in 58ad767da5cc2dd407e7fc28d9d1ef6f79aaf1ac.

ItielMaN commented 3 years ago

@lostindark Thank you! See the attached lang file, updated with the Boot Critical addition and the removal of the old strings. You may add it to the app now, but it'd be nice if you could fix the second and third issues mentioned in my last comment). Would you like me to file a different issue and close this one out, as you've fixed the major issues here already?

Language.he-IL.resx.zip

lostindark commented 3 years ago

I haven't figured out the text RTL issue above (2 & 3). @ItielMaN can you submit a pull request to add the language?

ItielMaN commented 3 years ago

Sure. Hopefully I'll have the time for this later today when I get back home.

ItielMaN commented 3 years ago

Sure. Hopefully I'll have the time for this later today when I get back home.

Done.

Fixed in 58ad767.

Thank you for that again, but this is not 100% fixing it. The text now is indeed RTL, but still aligned to the left. תמונה The "להמשיך?" in the second line should be aligned to the right.

lostindark commented 3 years ago

Fixed in 131eaf3bc782d111e8252a851893ede4408fe6ae.