Display emoji descriptions in popups

[eranl]

This is a draft for discussion.

Using descriptions downloaded from CLDR, an asset file per supported locale is created.
The script that creates these descriptions files depends on #1515 for the lists of supported emojis.
The descriptions files are bundled with the app. Should they be made into separate download(s)?
Using modified popups, these descriptions are then shown for emoji keys.
Some of the adaptations I made to popups seem a little hacky to me, so I'd appreciate advice for improving this.
This feature should probably be controlled by a setting.
Descriptions are shown in the current keyboard language. Should we use the main system language instead? I can see reasons to do either.
Emojis with flavors don't show descriptions, since the popups are taken. I'd appreciate ideas for fixing this.
Saving and restoring recent emojis loses the descriptions. To be fixed.
A ? artifact shows in the popups, which is weird. To be fixed.

Fixes #1532.

[Helium314]

The descriptions files are bundled with the app. Should they be made into separate download(s)?

Hm. Yes and no?
It's a lot of additional data (11.5 MB diff, 2 MB larger APK), which is too much for a feature no one appeared to be interested in until #1532.
But the descriptions probably don't work with a dicitonary (though I didn't try), when adding them to an emoji dictionary would be the most viable alternative.
But if it's not in dictionaries, asking users to download yet another thing seems a little excessive... still I thing bundling the descriptions with the app is too much.

Some of the adaptations I made to popups seem a little hacky to me, so I'd appreciate advice for improving this.

Popups are... weird. I understand they were made the way they are for performance reasons, but dealing with them can be awful.
Some description text is even too long for the popups.
It might be better to use some other view than a popup, like a TextView with some sort of border.

Descriptions are shown in the current keyboard language. Should we use the main system language instead? I can see reasons to do either.

I tend towards the keybord language, but no strong opinion here.

Emojis with flavors don't show descriptions, since the popups are taken. I'd appreciate ideas for fixing this.

Currently my only idea would be displaying the text in a separate line. But that would look rather awful, and you'd run into the "all popups are same size" problem.

[eranl]

Thanks for the feedback.

a feature no one appeared to be interested in until #1532.

I think most people don't know it's possible, because they haven't seen it anywhere. I certainly didn't know it was possible until I found CLDR.

asking users to download yet another thing seems a little excessive... still I thing bundling the descriptions with the app is too much.

So is this feature feasible? Maybe bundle descriptions with dictionaries?

[Helium314]

So is this feature feasible? Maybe bundle descriptions with dictionaries?

I don't know how they could be resonably bundled with the dictionaries without throwing overboard the idea of making dicitonaries for all AOSP keyboards.
Maybe there is a way to have them in the dictionaries?

like a TextView with some sort of border.

Actually if such a TextView is positioned properly, it could be made to appear above the popups, so you simply display both.

[eranl]

Maybe there is a way to have them in the dictionaries?

Based on this, I don't see how. Do you have an idea for doing that?

Actually if such a TextView is positioned properly, it could be made to appear above the popups, so you simply display both.

So either one or both of these would show up on long tap? Should I make any changes to the hint?

[Helium314]

Do you have an idea for doing that?

Currently my only idea would be trying to mis-use the shortcut, like word=👋🏻 and shortcut=this is a hand. But I didn't try, maybe it doesn't even compile, maybe it causes issues somewhere else.

So either one or both of these would show up on long tap?

I'd say both, so on top you have the view with the description, and below the usual popup keyboard.

Should I make any changes to the hint?

If this is added via setting, I think the users should be aware and a hint might not be necessary.

[eranl]

I'd say both, so on top you have the view with the description, and below the usual popup keyboard.

Either one of these may not be available for any given emoji.

If this is added via setting, I think the users should be aware and a hint might not be necessary.

The thing is not all emojis have descriptions available.

[Helium314]

The thing is not all emojis have descriptions available.

Is this still valid with the CLDR descriptions? I'm considering packaging at least the English dictionary with Heliboard, which could be used as fallback. (or do users actually expect descriptions for the "emoticons" section?)

Either one of these may not be available for any given emoji.

When there are no popups the description text could just be placed lower?
And both not existing just shows nothing, right?

[eranl]

The thing is not all emojis have descriptions available.

Is this still valid with the CLDR descriptions?

Yes, not all emojis have CLDR entries, and not all entries have tts .

When there are no popups the description text could just be placed lower? And both not existing just shows nothing, right?

Done.

[eranl]

Just committed a new implementation, which takes descriptions from a dictionary, and displays them using a new popup text view. It also adds a setting.

Questions:

1. Any idea about the build failure? I don't get it.
2. I made AndroidSpellCheckerService.createKeyboardForLocale public static in order to reuse it. Is there a utility class I can move it to?
3. Do I need to use exclusive numbered dictionary sessions? I just used 0 with no guard around it for now.
4. With description frequencies set to 255, checking suggestion results for KIND_WHITELIST or KIND_SHORTCUT and mScore > 0 seems to catch all true descriptions, and only a few non-descriptions. Anything I can do to filter the latter out?
5. I'm wondering if this is a bug or a feature: after you long-press an emoji and a description (only) pops up, the emoji does not get inserted. This is nice if you're checking around different emojis before you decide on one, but may be unexpected. It's also inconsistent with the other cases (flavors only, both flavors and description, or none), where the emoji does get inserted.

[Helium314]

1. my fault, I had put the wrong hash in gradle-wrapper.properties
2. why do you create another keyboard?
3. I never worked with sessions for the dictionaries. I guess this might be necessary when there are parallel calls to getSuggestions on the same dictionary? For testing you could try triggering issues by doing multiple calls in parallel on the same sessionId. Or you could just leave it at 0 and we deal with it in case we encounter problems.
4. which dictionaries are you querying?
5. hmm... when the setting is opt-in we could assume the user wants to check descriptions and not inserting is definitely perferable. But as it's now I'm not sure.

[eranl]

why do you create another keyboard?

What's the alternative?

I guess this might be necessary when there are parallel calls to getSuggestions on the same dictionary? For testing you could try triggering issues by doing multiple calls in parallel on the same sessionId. Or you could just leave it at 0 and we deal with it in case we encounter problems.

I did notice some weird cases with rapid successive lookups, where one lookup seemed to 'bleed' into another. Where do I get info about how many sessions I should have?

which dictionaries are you querying?

I guess all, as I didn't see a way to limit it. But the matches seem to all come from the emoji dictionary - they seem to be manufactured from other entries.

[Helium314]

What's the alternative?

I haven't properly looked through the code yet, but my idea way getting some kind of offset from the popup keyboard (maybe whether popups are displayed or not is already enough). The offset could be added to the view.
I'll need to find some time for review before I can give a useful comment on this.

I did notice some weird cases with rapid successive lookups, where one lookup seemed to 'bleed' into another. Where do I get info about how many sessions I should have?

Sorry, I don't have any experience on this. You will need to test and find something that works.

I guess all, as I didn't see a way to limit it

It should really only be the emoji dictionary. I think at some other place I told you of my (halfway done) plan to have some kind of DictionaryFacilitator (or at least something similar) that works with a single dictionary. This would be ideal for what you need.

[eranl]

my idea way getting some kind of offset from the popup keyboard (maybe whether popups are displayed or not is already enough). The offset could be added to the view.

What can I use for the keyboard parameter here?

my (halfway done) plan to have some kind of DictionaryFacilitator (or at least something similar) that works with a single dictionary.

Please let me know when it's ready, or if I can help with that.

[eranl]

Is it a bug that adding/removing/replacing a dictionary requires an app restart?

[Helium314]

Is it a bug that adding/removing/replacing a dictionary requires an app restart?

That would be a bug.

Please let me know when it's ready, or if I can help with that.

It's added in e034065 (original intention was the thing that will be using the suggestionLogger)

What can I use for the keyboard parameter here?

https://github.com/Helium314/HeliBoard/blob/main/app/src/main/java/helium314/keyboard/latin/SingleDictionaryFacilitator.kt#L20

[eranl]

https://github.com/Helium314/HeliBoard/blob/main/app/src/main/java/helium314/keyboard/latin/SingleDictionaryFacilitator.kt#L20

Thanks. How do I get a Dictionary to pass to it?

[Helium314]

Thanks. How do I get a Dictionary to pass to it?

You can use DictionaryInfoUtils.getCachedDictsForLocale and then create a ReadOnlyBinaryDictionary from the file. See DictionaryFactory.checkAndAddDictionaryToListNewType for an example.
If the dictionary is in assets, you need to extract it first using checkAndAddDictionaryToListNewType, then it's found by getCachedDictsForLocale.

[eranl]

after you long-press an emoji and a description (only) pops up, the emoji does not get inserted.

I fixed this inconsistency - now the emoji always gets inserted after a long press.

I did notice some weird cases with rapid successive lookups, where one lookup seemed to 'bleed' into another.

This is still a (rare) problem. An example: if I long-press judge, then man judge, the popup will show judge. If I long-press a second time, the popup is correct, but then it will bleed to the next popup. In other cases, no description is shown on the first long press after seeing a description of a different emoji.

Before switching to SingleDictionaryFacilitator, I experimented with random session ids, and that fixed the bleeding. But I don't know the semantics of sessions.

You can use DictionaryInfoUtils.getCachedDictsForLocale and then create a ReadOnlyBinaryDictionary from the file. See DictionaryFactory.checkAndAddDictionaryToListNewType for an example.

Thanks. Done. I refactored DictionaryFactory.checkAndAddDictionaryToListNewType in order to reuse the logic. It's less efficient now in the case of multiple dictionaries for a locale-type combination, but that's rare, right?

If the dictionary is in assets, you need to extract it first using checkAndAddDictionaryToListNewType, then it's found by getCachedDictsForLocale.

This is done on startup, right?

[Helium314]

Thanks. Done. I refactored DictionaryFactory.checkAndAddDictionaryToListNewType in order to reuse the logic. It's less efficient now in the case of multiple dictionaries for a locale-type combination, but that's rare, right?

It's rare, and I heavily doubt the change can a relevant impact on performance at all.

This is done on startup, right?

Dictionaries are extracted only when they are needed, i.e. we have a dictionary in assets with a type that is not available in the cached dictionaries (DictionaryFactory line 37).

This is still a (rare) problem. An example: if I long-press judge, then man judge, the popup will show judge. If I long-press a second time, the popup is correct, but then it will bleed to the next popup. In other cases, no description is shown on the first long press after seeing a description of a different emoji.

I'll have a look at this, but I also have no experience with session ids and will need to investigate.

[eranl]

Dictionaries are extracted only when they are needed, i.e. we have a dictionary in assets with a type that is not available in the cached dictionaries (DictionaryFactory line 37).

Am I guarantied that this has been done by the time the emoji keyboard is shown, or do I have to trigger it? How?

[Helium314]

Am I guarantied that this has been done by the time the emoji keyboard is shown, or do I have to trigger it? How?

Well, the file is extracted after DictionaryInfoUtils.extractAssetsDictionary has finished. So if you call it before showing the emoji keyboard then yes. But you will need to call it yourself when you want to have a dicitonary file that's only on assets.
Note that this call does now return null on failure to extract, related to #801 (but only should happen before device is unlocked).

[eranl]

Not sure I follow. You mean that if we add an emoji dictionary to assets, and the user doesn't add it to their list of dictionaries, I'll have to do that?

[eranl]

Summary of the open comments:

• mDictionaryFacilitator.getWordProperty(emoji); can be null, a check should be added

• maybe loadKeyboard could be renamed to reloadMainKeyboard? But no strong opinion, so if you think loadKeyboard is better then let's leave it

• rename OnKeyEventListener to EmojiViewCallback (or something similar)

• make sure that the facilitator is not reloaded unnecessarily, and that it gets closed properly (i.e. is not left open when the view is destroyed)

Done.

[eranl]

1. Noticed that the emoji popup has no vertical offset above the key like other popups, which makes it hard to see, so added some. The main keyboard view uses mKeyPreviewDrawParams.getVisibleOffset(), which is not easily accessible from EmojiPageKeyboardView (should I expose it?), so I used getKeyboard().mVerticalGap instead.
   BTW, I found that mVisibleOffset is set by the first key preview, so if you switch to numpad, which has no previews, before seeing any previews, there's no popup offset, which seems like a bug.

Would appreciate your comments about these.

[Helium314]

I remember noticing the inconsistent behavior quite a while ago, but never found the time to investigate (I guess I still have it on some list).
Having consistent behaviour would be appreciated, but I think it should be a separate PR.

[Helium314]

2 minor things left:

• DictionaryFactory.getDictionary(dictFile, locale) can still return null, e.g. with a bad dictionary file.
• Tthe dictionary facilitator is now static, so it should not use the m that indicates instance variables. Choose whether you use s or not, I just find the m misleading here.

(I'll go though whatever came up in the last 2 days and hope to merge this tomorrow at latest)

[eranl]

• DictionaryFactory.getDictionary(dictFile, locale) can still return null, e.g. with a bad dictionary file.

Fixed.

• Tthe dictionary facilitator is now static, so it should not use the m that indicates instance variables. Choose whether you use s or not, I just find the m misleading here.

Fixed.

Before releasing this, the dictionaries have to be made available and added to dictionaries_in_dict_repo.csv, right?

[Helium314]

Before releasing this, the dictionaries have to be made available and added to dictionaries_in_dict_repo.csv, right?

Definitely, and the dictionary screen needs to be adapted so users can disable the emoji dictionary for normal suggestions (or maybe set the weight, with 0 just disabling the dictionary).
But for that there is enough time between 3.2 and 3.3 beta.

[eranl]

Are you planning all of that for a separate PR? Which part can I start working on?

[Helium314]

dictionaries_in_dict_repo.csv is updated right before a release.
So what needs to be done before the first 3.3 beta is

• merge this PR
• update the dictionary screen to expose the weight (there is some todo in DictionaryFactory I think)
• finish the dictionary PR
• and I think the inline emoji search should also be added in 3.3

So all you can start working on is the dictionary screen update, as you're already working on the rest.

[sudoshindo]

🎉🎉🎉🥳

[eranl]

Dictionary loading will not work for all languages. Should I add this change in a separate PR?

[eranl]

• update the dictionary screen to expose the weight (there is some todo in DictionaryFactory I think)

Should this be done in a separate PR or in the emoji search one?
How about adding a global boolean Use emoji dictionaries for regular suggestions setting?

[Helium314]

Dictionary loading will not work for all languages. Should I add this change in a separate PR?

I don't think locales should be mixed that way. If a dictionary gets added to an incorrect directory, then I think it's better to fix adding dictionaries than trying to deal with this issue.

Should this be done in a separate PR or in the emoji search one?

In a separate PR, because it's not only for emoji dictionaries but for any dictionary.

How about adding a global boolean Use emoji dictionaries for regular suggestions setting?

You mean instead of setting a weight, or additional? With additional allowing users to disable emoji suggestions in 2 different ways can be confusing. Though we could make the globar switch change the weight, so things should be clear enough.

[eranl]

I don't think locales should be mixed that way. If a dictionary gets added to an incorrect directory, then I think it's better to fix adding dictionaries than trying to deal with this issue.

What do you mean by mixed? Isn't this normal locale fallback logic? What's the fix you're referring to?

You mean instead of setting a weight, or additional?

I meant instead, as a simplification. If you prefer the general weight approach, I'd be happy to implement, but would need some guidance about the details you're planning.

[Helium314]

What do you mean by mixed? Isn't this normal locale fallback logic?

Dictionaries should always be in the cached folder of the locale they were added to. This is how it already works when extracting assets dictionaries, and I think it should stay that way. The fallback logic is a bit more more exhaustive (using the bestMatch) and decides which assets dictionary is extracted into the cache folder for the current locale.

What's the fix you're referring to?

I don't know the bug, so I can't tell you. But you imply that dictionaries are added to the wrong locale for some reason, and that should not happen.

I meant instead, as a simplification. If you prefer the general weight approach, I'd be happy to implement, but would need some guidance about the details you're planning.

Weight has the advantage that people can adjust how many emojis they get with the normal dictionary, instead of being stuck with f <= 20. Might also be useful for some other addon dictionaries.
But there is certainly no need to add it, we can also start with the simple setting.

[eranl]

I don't know the bug, so I can't tell you. But you imply that dictionaries are added to the wrong locale for some reason, and that should not happen.

As I described here, the Lao (Laos) language's locale in HeliBoard is lo_LA, and the CLDR-based dictionary is for lo.

we can also start with the simple setting.

What's the better approach - when the new setting is off:

• not load the emoji dictionary into the mainDict collection?
• or set the emoji dictionary's weight to zero?

Either way, the logic for determining if the dictionary group has to be reloaded, and managing the reloading, becomes more complex. Do you have any tips for that?

[Helium314]

As I described here, the Lao (Laos) language's locale in HeliBoard is lo_LA, and the CLDR-based dictionary is for lo.

Then the fix is not auto-selecting a dictionary locale when it's not available in the select locale dropdown when adding a dictionary. (done in 77a728e)

What's the better approach - when the new setting is off

My idea with using weight was to not add a dictionary to the collection when the weight is 0. Since we'll start without weight anyway, the first approach would be the way to go.

Either way, the logic for determining if the dictionary group has to be reloaded, and managing the reloading, becomes more complex. Do you have any tips for that?

Without having considered everything: my first idea would be just skipping dictionaries of type emoji in checkAndAddDictionaryToListNewType (which I noticed should be ...ToListIfNewType) if the setting is off.
Since this is used only when creating the main collection it should not influence getting emoji dictionaries for search.

managing the reloading

I'm not sure what exactky you are referring to? When to reload the collection?

[eranl]

The main issue with dictionary reloading is that it relies on isInitialized, which is now not a sufficient condition - we now have to check if the group contains an emoji dict, which is hidden by the mainDict collection.

[Helium314]

I don't really understand. Is this something for the current state of Heliboard, or for the emoji search PR?

[eranl]

we can also start with the simple setting.

Either way, the logic for determining if the dictionary group has to be reloaded, and managing the reloading, becomes more complex. Do you have any tips for that?

I'm not sure what exactky you are referring to? When to reload the collection?

The main issue with dictionary reloading is that it relies on isInitialized, which is now not a sufficient condition - we now have to check if the group contains an emoji dict, which is hidden by the mainDict collection.

This is a summary of our discussion. When the user changes the new setting, we have to reload the dictionary group. In order to detect that, we have to check if the group contains an emoji dict.

[Helium314]

We can just reload the dictionaries anyway, it's not like the user will change settings every few minutes.