Skip to content

Conversation

david12341235
Copy link

First time contributor checklist:

Contributor checklist:

  • My contribution is not related to translations.
  • My commits are in nice logical chunks with good commit messages
  • My changes are rebased on the latest main branch
  • A pnpm run ready run passes successfully (more about tests here)
  • My changes are ready to be shipped to users

Description

Previously, text selection in conversation history would fail if selection started on an emoji, as described in #7206. This change addresses the issue with a small CSS style addition.

Manual testing - sent messages containing emojis with two standalone profiles and reproduced. Tested on other occurrences of emojis across the app such as story reactions and Chats sidebar previews to ensure unintended selection isn't possible.

No new tests, ran on OSX Sequoia 15.6.1, no other devices tested.

Look forward to contributing more, thought I'd start with a quick fix to get the hang of things :) thanks for your work Signal team.

@jamiebuilds-signal
Copy link
Member

I think this would improve selection in more places actually, like conversation titles and descriptions.

In retrospect, I think we can just make the .FunInlineEmoji element user-select: inherit from the surrounding text. Would you mind updating your PR to make this change? I'd like to give you credit in the commit

  .FunInlineEmoji {
    /* ... */
-   user-select: none;
+   // We want to inherit here since most usage of <Emojify> should not be
+   // selectable, but in cases like messages, conversation titles/descriptions,
+   // etc we want emoji to be selectable within the surrounding text.
+   // If the root `.FunInlineEmoji` element isn't selectable, then selection
+   // can't start on the emoji itself.
+   user-select: inherit;
  }

user-select: inherit is the default value but I want to make sure no one comes by and changes it to user-select: none again in the future, good to have this documented in a comment too.

@jamiebuilds-signal jamiebuilds-signal self-assigned this Sep 16, 2025
@david12341235
Copy link
Author

@jamiebuilds-signal sure thing, just made the changes (and signed the Contributor License Agreement). Thanks!

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.

2 participants