-
Notifications
You must be signed in to change notification settings - Fork 78
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
System to pick a translation language for each keyboard #476
base: main
Are you sure you want to change the base?
Conversation
Removed unnecessary function from project and simplified a switch case: Are these required or is it fine to remove them?
- Minor refactor to code for the button - Reworked how the chevron is displayed on the button - Added a label to the left of the chevron - Resized elements of the button to be centred instead of slightly lower than the centre - Added a button to language settings for picking a translation language - Created temporary value to store currently selected translation language, the plan is for every keyboard to have one of these
+ in progress class for the new view
- Selection now changes translationLanguage variable - Radio icons change based on selection
- Now saves a specific translation language for each keyboard instead of one value - Simplified saving the variables for each language using UserDefaults
- Translation query now asks for the controller language's specified translation language or sets a default if it's missing - Removed some unnecessary code
Thank you for the pull request!The Scribe team will do our best to address your contribution as soon as we can. The following is a checklist for maintainers to make sure this process goes as well as possible. Feel free to address the points below yourself in further commits if you realize that actions are needed :) If you're not already a member of our public Matrix community, please consider joining! We'd suggest using Element as your Matrix client, and definitely join the General and iOS rooms once you're in. Also consider joining our bi-weekly Saturday dev syncs. It'd be great to have you! Maintainer checklist
|
cd28e5e Merge pull request scribe-org#25 from weblate/weblate-scribe-scribe-i18n dc922d8 Translated using Weblate (Swedish) 997460b Translated using Weblate (Spanish) a99deb4 Translated using Weblate (German) eff6dee Merge pull request scribe-org#24 from Jag-Marcel/new-strings 7ad62b4 Merge branch 'main' into new-strings b020823 Merge pull request scribe-org#23 from weblate/weblate-scribe-scribe-i18n 463dd85 New strings for Scribe-iOS translation picker dde9e54 Translated using Weblate (Swedish) 65d4274 Translated using Weblate (Swedish) f2c989d Translated using Weblate (Swedish) d230853 Merge pull request scribe-org#22 from Jag-Marcel/minor-fix dc219d4 Minor fix to conversion script git-subtree-dir: Scribe/i18n git-subtree-split: cd28e5e4ad8f50026e6e4d5b63e489819152e1e6
d9b2226 Merge pull request scribe-org#26 from Jag-Marcel/action-fix 0cb65ed Small fix to action for converting jsons to xcstrings git-subtree-dir: Scribe/i18n git-subtree-split: d9b22262f3769545a08bd4772cae20caa2f23c67
Thanks for the detailed PR and fixing the merge conflicts, @Jag-Marcel! |
Brought in the localizations to main so that we can get v3.1 out, @Jag-Marcel 😊 Would be great if we can get a "Follow us on Mastodon" localization key in! I'll bring in everything else and send along v3.1 now though as I'd like to get it out today. No stress on one menu option not being fully localized :) |
@Jag-Marcel, the most recent commit adds in a table for each language with a |
Previous changes were broken because Utilities.swift was removed from several places as a source
Smaller fixes: - Changed localization keys for new feature to use same ones as Scribe-i18n - Removed extra English.entitlements in directory root
Should be done as soon as we have the full tables now! |
Nice, @Jag-Marcel! This is amazing stuff here :) I'll do a review as soon as I can. Let me know if there's anything else you'd have interest in looking into over the last week 😊 |
@Jag-Marcel, playing around with it a bit, and let me just say that you really have a knack for user experience :) My thought was always that the user would select a new radio button and then go back to the settings via the back button, but then given the UI they will see the change in the interface option, so it makes total sense that we take them back directly. As with the commands on highlighted words, well done 👏 |
We do have some keyboard key spacing issues that are just the damn conditionals for language getting mixed as always. Might be something that we should consider setting up an enum for at some point, but as you can see there's a space being applied after the This is the German keyboard with Spanish translation language :) Padding code in general starts here. For the German keyboard there are spacing issues for:
The issue above is more key size than padding, which is in KeyboardKeys.swift#L212. Might need to check these sorts of things for iPads as well. Any thoughts on this? An enum would be ideal, or maybe a variable for the translation language itself so that we're not checking between what the keyboard language is and whether translation is in use (not what the translation language actually is). |
I'm also a bit confused on the order of the translations. When I'm translating from Spanish -> German I'd expect to be able to do Ah I see what might be going on here (pardon the stream of conscious). We have it setup where it's from Let me know if this makes sense! Hope I'm understanding what I'm seeing from the translation results 😊 Will look into the code a bit more later. Happy to discuss or hop on a call if need be! |
I'll look into the first issue, maybe a function that returns the controller language's translation language is good? That could also simplify other places where it's used. |
Turns out that function already exists, I just forgot I made it at some point 🙃 |
Looking forward to the changes, @Jag-Marcel! :) |
Mistook something in the re-activation of the English keyboard
- Refactored some logic to be more brief & consistent - Added checks for the translation keyboard so spacing and key size is consistent with regular version of the keyboard - Simplified code to use translation language code function - Fixed a bug that crashed iPad version of the app
@andrewtavis just committed what I have for changing the database behaviour so far, but there seems to be a problem reading data from other databases. I added all the keyboards as memberships for each database, but in the reading process they seem to never find the word I'm looking for. Is there maybe something here I missed that makes the DBs show as empty unless they're used by the correct keyboard? |
Hmmmm, ya this could prove to be a problem. Maybe the structure of the database is off? I understand your original inclination on this to use one DB for the whole thing rather than go across multiple, which also makes sense given downloading data in the future. As I'm thinking, we'd now need to bring down the I'll take a look at this later! How are we on the layouts? Thanks for all the efforts, @Jag-Marcel! |
The layouts should be done, I'll try looking into the DBs some more as well. |
That's a thought as well, @Jag-Marcel! Could be something in the language keyboards base where we have all the tables in one names after their source language or something like that. If we can't figure out the current implementation, then I can rework the process :) |
Very sorry for the wait on all this, @Jag-Marcel! Thanks so much for your patience :) d4e2b0a sends along new language DBs without a translation table, and then there's a common How do you want to proceed here? Would you want to finalize the new process via the SQLite access of the new |
@andrewtavis I'll look into this and tell you if I need anything, glad that the process and everything is finally figured out! |
Contributor checklist
Description
This PR creates a menu for each keyboard, where you can choose a language to be used for Scribe's translation feature. Previously, every keyboard would require English as the translation language.
Related issues
Blocked by
[language abbreviation]_translations
as the name for the tables, but could still be changed