Contributing to Mattermost - I said ":D", not "😄"

#opensource

I'd like to talk about an issue I've been working on for the past couple of weeks in Mattermost - an open source alternative to Slack.

mattermost / mattermost

Mattermost is an open source platform for secure collaboration across the entire software development lifecycle..

Mattermost is an open source platform for secure collaboration across the entire software development lifecycle. This repo is the primary source for core development on the Mattermost platform; it's written in Go and React and runs as a single Linux binary with MySQL or PostgreSQL. A new compiled version is released under an MIT license every month on the 16th.

Deploy Mattermost on-premises, or try it for free in the cloud.

Learn more about the following use cases with Mattermost:

Other useful resources:

Download and Install Mattermost - Install, setup, and configure your own Mattermost instance.
Product documentation - Learn how to run a Mattermost instance and take advantage of all the features.
Developer documentation - Contribute code to Mattermost or build an integration via APIs, Webhooks, slash commands, Apps, and plugins.

View on GitHub

The issue in question requires me to add a client setting that allows users to toggle the automatic rendering of emoticons (:D) as emojis (😄).

Add a setting to disable emoticon rendering in messages on the client side #26504

mattermod posted on Mar 18, 2024

We want to add a user setting to disable the rendering of rendering of emoticons (:p, :D, etc) as emojis in the web and mobile app. That setting will affect how the current user sees the emoticons, and it will not affect how other users see them.

More context here:

Design Notes Figma link Add a User Configuration under Settings > Display that will enable end users to configure if they see text emoticons in all (sent and received) messages as plain text :P or rendered as emojis 😛

If you're interested please comment here and come join our "Contributors" community channel on our daily build server, where you can discuss questions with community members and the Mattermost core team. For technical advice or questions, please join our "Developers" community channel.

New contributors please see our Developer's Guide.

JIRA: https://mattermost.atlassian.net/browse/MM-53650

View on GitHub

Discovering The Issue

This feature was originally requested way back in 2016 and has since then been tossed around in a bunch of issues and "feature idea" forum posts. Here's a timeline:

January 25, 2016: First GitHub issue

way to turn off emoticons? #1982

Pomax posted on Jan 25, 2016

Skype etc. allow you to turn off emoticons, so that you get the raw text input instead of the converted smiley face. Not everyone appreciated chat filled with emoji, so it'd be a nice feature to have emoticon rendering a setting people can turn on or off

View on GitHub
February 20, 2016: Feature Idea forum post

way to turn off emoticons? – Mattermost Feature Proposal Forum

mattermost.uservoice.com
July 12, 2023: Second GitHub issue

Emojis are displayed when they shouldn't #24005

TehPeGaSuS posted on Jul 12, 2023

This issue was closed, asking to open a feature idea which was opened here.

This feature idea was opened more than 7 years ago and has received 146 votes so far.

But, unfortunately, nothing has been done to solve this issue.

The emoji replacement breaks code examples when not inside backticks, etc, being in overall extremely annoying.

Has someone stated, this is just silly.

Please consider changing the emoji replacement behaviour, since we have a button exactly for that on the input field.

Best regards!

View on GitHub
March 18, 2024: Third GitHub issue
Add a setting to disable emoticon rendering in messages on the client side #26504
mattermod posted on Mar 18, 2024
We want to add a user setting to disable the rendering of rendering of emoticons (:p, :D, etc) as emojis in the web and mobile app. That setting will affect how the current user sees the emoticons, and it will not affect how other users see them.

More context here:
- https://community.mattermost.com/private-core/pl/ntx55zpat7ds7gepnwbdci99eo
- https://github.com/mattermost/mattermost/issues/24005
Design Notes Figma link Add a User Configuration under Settings > Display that will enable end users to configure if they see text emoticons in all (sent and received) messages as plain text :P or rendered as emojis 😛

If you're interested please comment here and come join our "Contributors" community channel on our daily build server, where you can discuss questions with community members and the Mattermost core team. For technical advice or questions, please join our "Developers" community channel.

New contributors please see our Developer's Guide.

JIRA: https://mattermost.atlassian.net/browse/MM-53650
</div> <div class="gh-btn-container"><a class="gh-btn" href="https://github.com/mattermost/mattermost/issues/26504">View on GitHub</a></div>
I saw lots of people saying they wanted this feature implemented, which inspired me to pick it up.

Here's an example of the auto-rendering problem that I found hilarious - linked by @szi-secunet on GitHub in the original issue:

Later, while I was talking about the issue with my professor, he told me he'd worked with the person who filed the first issue way back in 2016, which further motivated me to try and see it through.

Starting On The Issue

Picking this issue up, I expected a challenge - I figured there must be a reason the issue had gone unattended for so long. In addition, the person who originally picked this issue up was unable to complete it, and this would be my first time contributing to such a large project.

While the issue filed on GitHub described adding the feature to both the web app and the mobile app, I decided I'd ease into the issue and just try to implement it in the web app for now. In hindsight, not the smartest move - I didn't know their policy for partially closing issues, and for some reason I didn't think to ask, so I wasn't even sure they'd accept my work, but I figured the issue was important enough to try.

Before asking to be assigned to the issue, I thought I'd try and see if it was something I could feasibly implement. I set up the development environment and got the app built and running. I then tried to find the code responsible for rendering emoticons as emojis. I searched the repo for the word "emoticon" and found the util function that handled the rendering. I disabled the block of code that handled emoticons and sent a couple of test messages, and sure enough, it worked.

Making Progress

At this point I reached out to the maintainers on their own Mattermost server (my professor later pointed out that this is an example of "dogfooding", or using your own product as a means of testing and quality control). I told them I was looking into the issue for the web app and had found the code responsible for rendering emoticons as emoji. I asked for directions on adding user settings and asked what scope this should be handled at i.e. whether they wanted me to check for the setting directly in this util function or in the component that calls it. The function handled rendering both text-based emoticons (:D) and emoji (:grinning:), so I would have to refactor it to separate the logic. I later chose not to do this and keep my changes as simple as possible while I focused on getting it working.

One of the core maintainers responded, saying I was on the right track with the util function I'd found, and pointed me to an existing user setting to use as reference for adding one. They told me I had to create a new functional component for my setting, but looking into the existing user display settings, they were all created inside a parent component instead of as their own components. I left a message asking about it, but I wouldn't hear back for a while since it was the weekend. I decided a good approach would be to implement it within the parent component for now and then move it to it's own component after confirming with the maintainers.

Getting Stuck

Adding the user setting turned out to be pretty tricky. I had to work with several technologies I'd never used before - Redux, React Testing Library, and React class components. I was able to get it working by adding it within the parent component but even I could tell my approach was less than ideal - I had bits of code where I didn't even fully understand what they were doing. I realized at this point it'd probably have been easier to just make a separate component, because I could easily identify the bits of code I actually needed.

I heard back from the maintainer, confirming I should write a separate component - they said the code in that part of the project is pretty old and they wanted to improve it by splitting things up.

I was also told my new component should be a functional component, but then I realized the component I was told to reference was a class component. I looked around the user settings for an example of a functional component, but couldn't find one. I worked around this by picking out hunks of code from the reference component piece by piece, understanding what they did, and then translating that logic to a functional component.

Adding Tests

At that point, I got the setting working, and I recorded a short demo to show the maintainers. But I still had to update the unit tests and write new ones for my component. This took me a couple of days as I struggled with the long test suite (until I realized I should probably run the test watcher instead of the entire suite) and learning to test React components.

The Pull Request

Finally, a week after I began working, I made my pull request. You can find the demo of the working setting included:
[MM-53650] Add disable emoticon rendering setting to webapp #29414
uday-rana posted on Nov 28, 2024
Summary

This pull request adds a user setting to the webapp to toggle rendering emoticons (:D) as emojis (😄).

The setting is added as a component in components/user_settings/display/render_emoticons_as_emoji/ which is imported in components/user_settings/display/user_settings_display.tsx.

I've added a renderOnOffLabel() function to user_settings_display.tsx, lifted from components/user_settings/advanced/user_settings_advanced.tsx to help render the new component.

The setting is stored as a user preference using the savePreferences() action.

I've added constants for the preference to utils/constants.tsx and webapp/channels/src/packages/mattermost-redux/src/constants/preferences.ts.

To actually use the setting, I've modified components/post_markdown to receive it's value as a prop, for which I've used getBool() and added a default value to the config. post_markdown passes this value down to Markdown on the options object, which then passes it down to utils/text_formatting.tsx, which finally passes the value to emoticons.tsx as a newly added parameter. emoticons.tsx checks whether the value is true and if it is, it transforms the emoticons into emojis.

I've updated affected tests and created unit tests for the new component. I've also updated the English translation file.

QA Test Steps

Navigate to User Settings.

Go to the Display category.

Find the section labelled "Auto-render emoticons as emoji" and click "Edit".

Toggle the setting and click "Save".

Emoticon rendering on messages sent by the current user and other users should be toggled client-side with the setting.

Ticket Link

Fixes (partially) https://github.com/mattermost/mattermost/issues/26504 Jira https://mattermost.atlassian.net/browse/MM-53650

Note the issue and ticket describe adding this feature to the mobile app as well, which this PR does not.

Screenshots

before after

Release Note

Added a new user setting to toggle rendering emoticons (:D) as emojis (😄)
View on GitHub
I'm still awaiting developer and QA review but next I'd like to try and implement the setting on the mobile app. I'll be working with React Native for the first time, so fingers crossed things go well.

That's it for this post, thanks for reading!