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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix uppercase Greek vowels popups #1984

Conversation

tsiflimagas
Copy link
Collaborator

@tsiflimagas tsiflimagas commented Jul 29, 2022

So, I gave this a shot, but it turned out I didn't know how to handle more than one popup with case_selector. Thus, I'm once again looking for an advice馃榿

Closes #1981

@patrickgold
Copy link
Member

You can put case selectors in the relevant section too, basically you can put a case selector in every spot there an auto text key also fits :)

@tsiflimagas
Copy link
Collaborator Author

Thanks, it now works fine! I initially tried some weird stuff and didn't think of the simplest.
One last thing which I just came to notice. It seems like I found out why Gboard doesn't insert 螜虉虂 and 违虉虂 but only their lowercase forms. These characters don't actually exist standalone in Unicode, instead they're a combination of the plain letter+dieresis+accent. Till now in FlorisBoard I had been using the correct label, but the code was actually just the plain letter. What's weird is that they handle this in AOSP keyboard, but not in Gboard. Anyway, is there any way at the moment to have a combination of letters in the layout, or should I go with what Gboard does with those two characters?

@patrickgold
Copy link
Member

Anyway, is there any way at the moment to have a combination of letters in the layout, or should I go with what Gboard does with those two characters?

You can use a MultiTextKey for this. Actually this would be the first proper use for it. Example (code points are exemplar, you need to insert the correct ones which compose the letter):

{ "$": "multi_text_key", "codePoints": [1, 2, 3], "label": "违虉虂" }

Note that multi text keys do not have an auto case mapper, so you need to use a case selector and put above snippet in for the upper variation.

@tsiflimagas tsiflimagas force-pushed the fix_greek_uppercase_vowels_accents branch from 3a7c86a to da6bc37 Compare July 30, 2022 19:30
@tsiflimagas
Copy link
Collaborator Author

Oh, great! I tried but it seems I'm doing something wrong, as it just outputs the letter without the other two characters, so I pushed it in order for you to spot the mistake easier. I can't quite spot it myself.

@patrickgold
Copy link
Member

I tried but it seems I'm doing something wrong, as it just outputs the letter without the other two characters

You are doing everything correct, the issue is that the editor instance and the composer don't support multiple code points... I now adjusted the logic and created PR #1989 to fix this circumstance. I tested the behavior and it inserted the 违虉虂 as intended for me. If you want you can test this out locally (check out my PR's branch and manually apply the commit you pushed above).

I still need to do some testing if I didn't break something else with this change in the composer, but I am too tired for it now so I will do this tomorrow, then I will merge in the change into master.

@patrickgold
Copy link
Member

patrickgold commented Aug 1, 2022

#1989 is now merged, so if you rebase your branch onto the latest master it should work flawlessly now.

@tsiflimagas tsiflimagas force-pushed the fix_greek_uppercase_vowels_accents branch from da6bc37 to d58d08c Compare August 1, 2022 11:54
@tsiflimagas
Copy link
Collaborator Author

Yes, it works great! Thank you 馃榿

Copy link
Member

@patrickgold patrickgold left a comment

Choose a reason for hiding this comment

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

Thanks for the rebase! I did check the changes again and they look good, all the Greek popups are now properly capitalized and inserted correctly.

One question though: For letters with only one popup (, , , and ) they previously only had a relevant popup, now they have a main popup. Is this change intended?

Co-authored-by: Patrick Goldinger <patrick@patrickgold.dev>
@tsiflimagas tsiflimagas force-pushed the fix_greek_uppercase_vowels_accents branch from d58d08c to 6355b93 Compare August 1, 2022 13:43
@tsiflimagas
Copy link
Collaborator Author

Is this change intended?

Lol, I actually don't know why I did that, nor noticed there was no reason to. Changed it back.

Copy link
Member

@patrickgold patrickgold left a comment

Choose a reason for hiding this comment

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

Thanks for the fix, now everything checks out fine. Will merge in the changes now.

@patrickgold patrickgold merged commit 3a3e362 into florisboard:master Aug 2, 2022
@tsiflimagas tsiflimagas deleted the fix_greek_uppercase_vowels_accents branch August 2, 2022 20:32
snaik20 pushed a commit to snaik20/florisboard that referenced this pull request Oct 8, 2022
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.

Broken uppercase accented characters in Greek layout
2 participants