Fix detection of keys on emoji popups with long descriptions

[eranl]

When an emoji with variants has a long description, which causes the popup keys to be justified to the right, keys were not detected correctly.

[Helium314]

How can I reproduce this issue?

[eranl]

Try the hand with index finger and thumb crossed emoji.

[Helium314]

I don't think I can reproduce your issue. Though a description of expected vs observed behavior, plus more information that allows me to see where the emoji and popup are located might help.

On my Android 10 phone I had to override the Android version that's used for emojis. Depending on what I use, the hand with index finger and thumb crossed emoji is either in the left column, or on second from right.

When it's in the left column, everything seems to work fine, but this is obviously not where you descripted an issue. Anyway, in this case there is the minor change with this PR that the "default" popup key usually isn't selected when not moving the finger after long pressing. This may also happen without the PR, but only rarely.

When the hand with index finger and thumb crossed emoji is in the second colum from the right, without the PR there is the already known offset between default popup and key on the emoji keyboard.
With the PR there seems to be an offset towards the right, and depending on where exactly on the key I long press, I may end up with two different skin tones, none of them being the tone of the "default" popup. This behavior appears to be very similar to #1789.
Note that things may look different for me, because on Android 10 the popups are a little wider due to the emoji being displayed as two separate icons.

[eranl]

For me, the hand with index finger and thumb crossed emoji is also in the second column from the right. Without the PR, the selected popup key is about 1.5 key widths to the right of my finger (initially, the rightmost key is selected). With this PR, the selected popup key is directly above my finger (initially, the second-from-right key is selected).

[Helium314]

initially, the second-from-right key is selected

This is an issue that should be fixed. I very much agree the offset is ugly and unnecessary, but switching from UI weirdness to unexpected long-press behaior is not a good deal.

I don't know how the rearrangement of the popup keys is done / triggered, but using it should solve the issue.

Anyway, in this case there is the minor change with this PR that the "default" popup key usually isn't selected when not moving the finger after long pressing.

Seems to happen because the initial coordinates may be outside the popup panel due to removal of coordinate translation.
Keeping the translation and subtracting mPopupKeysKeyboardView.getLeft() fixes this.

[eranl]

unexpected long-press behaior

Why is it unexpected? Does the order of skin tones matter?

Anyway, in this case there is the minor change with this PR that the "default" popup key usually isn't selected when not moving the finger after long pressing.

I can't reproduce this. Can you please share an example?

Seems to happen because the initial coordinates may be outside the popup panel due to removal of coordinate translation. Keeping the translation and subtracting mPopupKeysKeyboardView.getLeft() fixes this.

The problem is that in onLongPressed, mPopupKeysKeyboardView.getLeft() is not set correctly yet, IIRC.

[Helium314]

Why is it unexpected?

Without this PR, long pressing without moving always results in the same skin tone (light by default, or none if the user set a default tone). This is very predictable behavior.
With the change, the default skin tone when long pressing without moving is suddenly dependent on arrangement of emojis and length of description. This is not expected, and I'm fairly certain it's unwanted.

Does the order of skin tones matter?

That somewhat as well, but order is dependent on popup position already anway (and I don't see how to have constant order without running into other issues).

I can't reproduce this. Can you please share an example?

With this PR, that's the default until onMove is called for the first time.

[eranl]

light by default, or none if the user set a default tone

It's actually light by default, or none if the user set the default tone to light

order is dependent on popup position

I find this unexpected, and I just noticed it.

I don't see how to have constant order without running into other issues

Can you please explain? In contrast to alpha keyboard popups, I think having a consistent emoji popup order is more important than having a consistent default emoji popup key.

It's very tricky to fix both the offset and the default key. AFAICT, this issue only affects this one emoji, so maybe we can leave it as is?

[Helium314]

It's actually light by default, or none if the user set the default tone to light

I had intended something else, but apparently failed to check properly. Anyway, now this behavior is default and I wouldn't change it without a good reason.

Can you please explain?

You cannot have constant order, always the same skin tone on long press, and initially selected popup on top of the base key.

I think having a consistent emoji popup order is more important than having a consistent default emoji popup key.

I agree on this, but we need to take into account what people are used to. I find it even more important to avoid changes in behavior when no one is asking for it.

It's very tricky to fix both the offset and the default key. AFAICT, this issue only affects this one emoji, so maybe we can leave it as is?

When you use a different default skin tone there are many more affected. There even is some misalignment when the base key is in one of the 3 leftmost colums and the text is so long the right edge alignment kicks in.
I really wish I would have tested this when I suggested this alignment switch... I suspect this alignment is related to the issue the default popup (because the key re-positioning doesn't take it into account)

[eranl]

I find it even more important to avoid changes in behavior when no one is asking for it.

Don't you and I count as users? 🙂

I really wish I would have tested this when I suggested this alignment switch...

This gave me an idea. I reverted the previous commits, and cancelled the alignment, keeping the popup keyboard in its original location regardless of description length, which fixes both the offset and the default key.

There are multiple ways to align the description above the popup keyboard:

1. center when the default key is in the center, align to side when the default key is to the side (committed version)
2. always center
3. option 1 for long descriptions, option 2 for short ones.

Which one do you prefer?

[Helium314]

Don't you and I count as users? 🙂

Oh we definitely do! On this particular thing though no one asked for a change in several years (counting OpenBoard), and also you didn't seem to notice until I pointed it out.
I'm not personally opposed to changing the order, but with such changes it's always the problem that you can't get feedback until you do the change and people start complaining.

Which one do you prefer?

I didn't think it though, but I tested and like the committed version,.

There is one minor glitch with the key shown as selected (on the first longpress without moving the finger), when often a wrong key is initially selected.
When testing it looks like popupKeysPanel.translateX in EmojiPageKeyboardView.onLongPressed doesn't work properly. E.g. the coordinates are 590, 159, translated to 353, 224 (highlighting the wrong emoji), and then in the up event the coordinates are translated to 531, 224 (selecting the correct emoji).

As far as I understand, this happens because PopupKeysKeyboardView.translateX uses mOriginX, which is set in showPopupKeysPanel. But this is called only at the end of onLongPressed.
What confuses me is that your code doesn't touch this order at all... I tried adjusting the order in onLongPressed and it seems to work, but maybe you have a different / better solution.

[eranl]

There is one minor glitch with the key shown as selected (on the first longpress without moving the finger), when often a wrong key is initially selected.

I can't reproduce this, but changing the order makes sense. Done. Looks like I broke it in #1542.

I'm seeing a similar issue though: if I press the bottom part of the emoji key (especially with high Emoji view font scale values), no popup key is selected until I slide my finger up. Is that intentional?

[Helium314]

Is that intentional?

Definitely not.
It does not happen if the descriptions are off, and the description size changes with height scale. So maybe this change somehow needs to be considered. Not sure where though...

[eranl]

Fixed.

[Helium314]

Working great now, thanks!