Skip to content

Add ability to remove or clear recent emojis - #2603

Open
RHJihan wants to merge 8 commits into
HeliBorg:mainfrom
RHJihan:feature/remove-clear-recent-emojis
Open

Add ability to remove or clear recent emojis#2603
RHJihan wants to merge 8 commits into
HeliBorg:mainfrom
RHJihan:feature/remove-clear-recent-emojis

Conversation

@RHJihan

@RHJihan RHJihan commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Adds the ability to manage the recent emoji list:

  • Long-press an emoji in the recent category to reveal a delete (bin) button (like a popup key) to remove that emoji from recent list.
  • Long-press the recent category icon to clear all recent emojis.

The delete button coexists with the existing "show emoji description on long-press" feature and is scoped to the recent category only.

Fixes #860
Fixes #2209

I want to be transparent that this PR was implemented mostly with Claude Code, with me guiding the process and testing the results.

If it meets the project's standards, feel free to merge it. If not, please treat it as a starting point. I'm happy for maintainers to edit, refine or rework it as needed.

I'm sharing it so the work just doesn't stay on my machine. Thanks for taking a look!

@MiMoHo

MiMoHo commented Jul 20, 2026

Copy link
Copy Markdown

Not a maintainer, but I had a close look at the diff — the feature itself looks well scoped (nice that removal reuses the popup mechanism and that removeKey/clearRecentKeys go through saveRecentKeys). A few points you may want to address to improve the odds of merging:

  1. values-bn/strings.xml change: this edits the existing hidden_features_message translation, and per CONTRIBUTING ("Updating translations in a PR will not be accepted, as it may cause conflicts with Weblate") that will likely block as-is. Also, the new paragraph documenting the feature currently exists only in Bengali — it should be added to the English hidden_features_message in values/strings.xml (new/changed source text is fine), and the Bengali version then updated on Weblate.

  2. Clear-all without confirmation: long-pressing the recents tab wipes the whole recent list irreversibly, with only haptic feedback. An accidental long-press on a small tab icon is easy, and there's no undo. I'd suggest a confirmation dialog (or at least making single-emoji removal the only destructive gesture and putting clear-all somewhere more deliberate).

  3. Key.java copy constructor: the zero-column workaround changes behavior of the core Key class for every key constructed with popup keys, to handle an emoji-recents special case. It would be safer to normalize mPopupKeysColumnAndFlags where the remove-key is built (showRemoveKeyboard) instead of in the shared constructor — that keeps the blast radius inside the emoji code.

  4. Behavior change in recents: long-press in recents now shows only the delete button, replacing whatever popup the key had before (e.g. skin-tone variants). If that's intentional it's worth stating in the PR description; alternatively the delete key could be appended to the existing popups rather than replacing them.

  5. Nit: EMOJI_RECENT_REMOVE = -10000 is correctly inside the reserved INTERNAL_HELI range (-19999..-10000), but the declaration sits between the currency slots (-80x) — moving it next to SYMBOL_ALPHA (-10001) would make the range usage easier to see.

@RHJihan

RHJihan commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Behavior change in recents: long-press in recents now shows only the delete button, replacing whatever popup the key had before (e.g. skin-tone variants). If that's intentional it's worth stating in the PR description; alternatively the delete key could be appended to the existing popups rather than replacing them.

In the recents category each entry represents a specific emoji the user already picked, including whatever skin tone they chose at the time. Re-opening the skin-tone variants there doesn't really make sense, since the recent entry is the historical choice

@RHJihan
RHJihan force-pushed the feature/remove-clear-recent-emojis branch from a5543b5 to a5bb900 Compare July 20, 2026 12:29
@RHJihan

RHJihan commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Force-pushed to fix the commit author info on the previous push, which was wrong. No changes to the actual diffs.

@Helium314

Copy link
Copy Markdown
Collaborator

@RHJihan I wasn't happy with the style, in general where things were added just for this functionality, and especially the addition of a keycode that is valid but useless everywhere except in a popup in recent emoji view.

I worked on a different implementation that now allows a more generic callback for popup keys: a32c560
When testing this seems to work fine, can you confirm that?
The whole way of dealing with emoji keys is still overly complicated, but I'm willing to leave it until there is need to improve it.

@MiMoHo

Behavior change in recents: long-press in recents now shows only the delete button, replacing whatever popup the key had before (e.g. skin-tone variants). If that's intentional it's worth stating in the PR description; alternatively the delete key could be appended to the existing popups rather than replacing them.

Can you elaborate on how you tested to get this result? As @RHJihan said in recents there are no popups except for the descriptions (but those still work), so I am confused.

Clear-all without confirmation

Is it really easy to accidentally long-press on this? And even if so, I'd say the damage is rather small.

@MiMoHo

MiMoHo commented Jul 22, 2026

Copy link
Copy Markdown

Thanks @RHJihan for the work here, and @Helium314 for the alternative.

First, a correction to my earlier review — I was wrong about the skin-tone popups. Recent emojis are built with their popup keys dropped (dropPopupKeys = mIsRecents in DynamicGridKeyboard.addKey, which predates this PR), so long-pressing a recent never showed variants, only the description. Nothing is being replaced; a delete action is just added where there was none. Sorry for the noise there.

I prefer @Helium314's remove_recents approach — the generic popup-key callback, and having recent keys carry the bin as a normal popup key, is cleaner than a dedicated keycode.

Building on that: one thing both versions still share is that "clear all" is an unconfirmed long-press on the recents tab — an invisible gesture on a navigation tab, easy to hit by accident, no undo. I put together a small change on top of remove_recents that folds it into the same mechanism: the recents long-press popup gets a second key (clear all) next to the delete-this bin, so both run through the one popup + callback path and the destructive tab gesture goes away. No new keycode — the two keys are told apart by their icon, which also matches your point about not adding popup-only codes.

Diff on top of your branch: remove_recents...MiMoHo:2603-unified-popup

@RHJihan

RHJihan commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

@Helium314, thanks a lot for taking the time to rework this and for the cleaner approach.

I tested it out and it works well overall. One thing I noticed: after long-pressing the recent category icon, the emojis are actually cleared under the hood, but the view doesn't update right away, so they still show up until you switch tabs. I double checked this by trying to delete one of the "still visible" emojis afterward, and nothing happened, which confirms they were already gone.

debug log:

2026-07-23 12:09:37.636082 I test: add 😀
2026-07-23 12:09:37.636916 I test: set [😀]
2026-07-23 12:09:37.638892 D RichInputConnection: committing 2 characters
2026-07-23 12:09:37.667652 I LatinIME: onUpdateSelection: oss=0, ose=0, nss=2, nse=2, cs=-1, ce=-1
2026-07-23 12:09:38.266317 I test: add 🤣
2026-07-23 12:09:38.266954 I test: set [🤣, 😀]
2026-07-23 12:09:38.268840 D RichInputConnection: committing 2 characters
2026-07-23 12:09:38.293975 I LatinIME: onUpdateSelection: oss=2, ose=2, nss=4, nse=4, cs=-1, ce=-1
2026-07-23 12:09:40.526002 I test: set []
emoji.mp4

@MiMoHo

MiMoHo commented Jul 24, 2026

Copy link
Copy Markdown

I looked into this @RHJihan. The clear does work; it's a refresh issue plus a second latent bug:

  1. clearRecentKeys() refreshes via notifyItemChanged(getRecentTabId()), but that doesn't rebind the currently-visible ViewPager2 page — and long-pressing the recents tab icon happens while you're on the recents page. Single-emoji removal uses the same notify but only appears to work because it goes through a popup interaction that redraws the page; switching tabs fixes it because the recents page is then freshly bound. So clear-all needs to force the current page to rebuild, not just notify.

  2. Separately, DynamicGridKeyboard.removeAllKeys() clears mGridKeys but not mPendingKeys. Since onPageSelected flushes pending keys on tab change, an emoji typed while viewing recents survives a clear-all and reappears after switching tabs. Clearing mPendingKeys there fixes that.

I've tested both on-device (clear-all now clears the grid immediately, and the pending emoji no longer reappears after a clear + tab switch). I opened #6 against your branch.

@RHJihan

RHJihan commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @MiMoHo! Just to clarify, the issue is in @Helium314's implementation.

@MiMoHo

MiMoHo commented Jul 24, 2026

Copy link
Copy Markdown

You're right @RHJihan, thanks for the pointer. On the remove_recents rework, clearRecentKeys() is just RecentEmojis.clear() + removeAllKeys() with no view refresh at all, so the recents grid is never told to update — my earlier note described the notifyItemChanged path from the PR branch, which is a bit different. There's also a second, shared issue: removeAllKeys() clears mGridKeys but not mPendingKeys, so an emoji typed while the recents tab is open survives a clear-all and gets re-added by the next flushPendingRecentKeys(). Both apply to the rework, so it makes more sense to put the fix there than on the PR branch: #2696

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.

Add a "clear emoji history" button to the recent emoji list Add an option to remove certain emojis from the recent history

3 participants