Add Custom Emojis Support #877

makotech222 · 2022-12-11T22:34:19Z

This PR adds the custom emoji functionality from LemmyNet/lemmy#2419 (comment)

In the frontend, I've added the following:

Update post-listing to display a new markdown icon to open emoji picker

Update markdown-textarea to handle emoji-picker
Update app startup to build emoji list from siteres
Update Tribute component to include custom emojis
Tabify Site Settings page
Add Custom Emoji admin tab for add/edit/delete custom emojis. Clicking on emoji in picker will scroll to entry in table.

Deficiencies:

Bug on emoji-mart: (Bug: Can't instantiate two instances of picker with different picker options missive/emoji-mart#756). Functionally, this means the picker can't update when emojis are being changed, unless page is refreshed. Not a show stopper, just annoying.
Bug on emoji-mart: (Bug: Empty picker just renders a giant style tag missive/emoji-mart#757) Picker blows up if emoji list is empty. Very minor and worked around

Open Questions:

Better icon for 'save' in the table? Currently using 'edit' icon.
Is including Image Url column necessary? Allows user to link to external images, but takes up space. I assume most people will just upload images to their server.

Nutomic · 2022-12-12T13:12:27Z

I think a good solution would be to split the site settings page into different tabs.

dessalines · 2022-12-14T17:45:30Z

Question: is "Category" for emoji-mart? So these all wouldn't just go in a single "custom" category, but potentially many custom categories?

If we go this route, I think it would be prudent to add api endpoints for PUT/POST/DELETE of the custom emoji entities, rather than use Site.

Sounds good to me.

Currently, I am just adding a <script> tag to the server's html to load the emoji-mart library. Seeing as how we don't do this for any other library, I imagine we would want to change this.

Fairly sure you can define that custom library location in the package.json and it will download it to node_modules. If that doesn't work, you can reference the git repo, if the js file is hosted there.

makotech222 · 2022-12-14T18:17:23Z

Yeah, Category is something for emoji-mart. You can add emojis to custom categories and it will group them.

makotech222 · 2023-01-08T00:42:00Z

Okay with the latest commit, the emoji picker will place a markdown image, with its 'title' attr set to the shortcode. When the markdown renderer encounters an image, it will check if it is a custom emoji, and if so it will add the 'icon-emoji' css class, which limits the size of the image.

dessalines · 2023-02-06T19:39:49Z

I'm just getting around to testing this now. There doesn't seem to be a save button for adding the custom emojis.

makotech222 · 2023-02-06T20:10:37Z

Its the icon on each row next to the delete button. Its not a great icon, but couldn't find any better suiting ones.

dessalines · 2023-02-06T19:50:23Z

src/shared/utils.ts

+
+
+let custom_emojis: any[] = [];
+let custom_emojis_lookup: { [k: string]: CustomEmojiView } = {};


Use a typescript Map<X, Y> for this, and probably add it to the globals near the top. camelCase too.

src/shared/utils.ts

dessalines · 2023-02-06T19:55:17Z

src/shared/utils.ts

@@ -678,6 +673,104 @@ export function setupTribute() {
  });
 }

+
+
+let custom_emojis: any[] = [];


Type this strongly, and use camelCase for typescript vars. It should probably be moved to the top too, since its a global.

dessalines · 2023-02-06T20:02:53Z

src/shared/components/home/admin-settings.tsx

+                <button
+                  className={`nav-link btn ${this.state.currentTab == "emojis" && "active"
+                    }`}
+                  onClick={() => { this.handleSwitchTab("emojis"); }}>


Use linkEvent here. Check settings.tsx for an example.

dessalines · 2023-02-06T20:03:10Z

src/shared/components/home/admin-settings.tsx

+              <li className="nav-item">
+                <button
+                  className={`nav-link btn ${this.state.currentTab == "site" && "active"}`}
+                  onClick={() => { this.handleSwitchTab("site"); }}>


Same, use linkEvent.

dessalines · 2023-02-06T20:26:19Z

src/shared/components/home/emojis-form.tsx

+                                                htmlFor={index.toString()}
+                                                className="pointer text-muted small font-weight-bold"
+                                            >
+                                                {cv.image_url.length > 0 && <img style="max-width: 24px; max-height: 24px; display: inline-block;" src={cv.image_url} />}


Use a style in main.css

dessalines · 2023-02-06T20:27:04Z

src/shared/components/home/emojis-form.tsx

+                                                disabled={cv.id > 0}
+                                                value={cv.shortcode}
+                                                onInput={(s: any) =>
+                                                    this.handleEmojiShortCodeChange(s.target.value, index)


All these should use linkEvent

With linkEvent, I can't pass any parameters to the function. I can technically embed it in the html element, but that seems so much messier. I've already done same function calls in earlier PRs without linkEvent. From documentation, it just seems like its slightly faster?

To pass data to linkEvent, do:

onClick = {linkEvent( {some_data_here}, this.handleBleh )}

dessalines · 2023-02-06T20:27:52Z

src/shared/components/home/emojis-form.tsx

+                    <br />
+                    <button
+                        className="btn btn-sm btn-secondary mr-2"
+                        onClick={e => this.handleAddEmojiClick(e)}


You have a button to add a block, but no button to save the emojis to lemmy.

dessalines · 2023-02-06T20:31:03Z

src/shared/components/home/emojis-form.tsx

+            let data = wsJsonToRes<DeleteCustomEmojiResponse>(msg);
+            if (data.success) {
+                let custom_emojis = [...this.state!.custom_emojis.filter(x => x.id != data.id)];
+                this.setState({ custom_emojis });


You should also be updating the global emoji list / maps.

src/shared/components/home/emojis-form.tsx

dessalines · 2023-02-06T20:43:26Z

That icon looks like an edit. Once you turn it from a table into a bootstrap form, it'll probably be better to name the button Save for clarity.

makotech222 · 2023-02-11T20:39:05Z

That icon looks like an edit. Once you turn it from a table into a bootstrap form, it'll probably be better to name the button Save for clarity.

Is this okay?

Id prefer a better icon, as its takes up less width, but I couldn't find any in our icon set that worked properly.

makotech222 · 2023-02-12T04:10:35Z

Among many fixes today, i added pagination to the admin panel for emojis, to deal with performance issues with large size lists.

src/shared/utils.ts

dessalines · 2023-02-21T14:19:50Z

src/shared/components/home/admin-settings.tsx

-              {this.admins()}
-              {this.bannedUsers()}
-            </div>
+          <div >


Weird extra space, wonder how this is passing lint.

dessalines · 2023-02-21T14:22:47Z

src/shared/components/home/admin-settings.tsx

@@ -179,6 +209,11 @@ export class AdminSettings extends Component<any, AdminSettingsState> {
    );
  }

+  handleSwitchTab(i: AdminSettings, event: any) {


Use the convention for tab switching from settings.tsx, line 214 .

dessalines · 2023-02-21T14:24:01Z

src/shared/components/common/markdown-textarea.tsx

@@ -226,6 +228,7 @@ export class MarkdownTextArea extends Component<
            >
              <Icon icon="link" classes="icon-inline" />
            </button>
+            <EmojiPicker onEmojiClick={(e) => this.handleEmoji(this,e)}></EmojiPicker>


Use linkEvent

dessalines · 2023-02-21T14:25:15Z

src/shared/components/home/emojis-form.tsx

+    page: number;
+}
+
+class CustomEmojiViewForm {


Why is this a class and not an interface?

dessalines · 2023-02-21T14:32:02Z

src/shared/components/home/emojis-form.tsx

+                    >
+                        <thead className="pointer">
+                            <tr>
+                                <th style="width:70px;">{i18n.t("column_emoji")}</th>


Okay, but at the very least remove the hard-coded widths ( those aren't going to be correct for any device but the one you coded them from), and use bootstrap table directives like in communities.tsx . As I said though, tables are really terrible for skinny devices like phones, which is why we eventually need to get rid of that, and switch to using responsive design.

Bootstrap has responsive form examples, which is what this is, and we have a good amount of responsive forms in lemmy.

dessalines · 2023-02-21T14:35:43Z

src/shared/components/home/emojis-form.tsx

+                                                disabled={cv.id > 0}
+                                                value={cv.shortcode}
+                                                onInput={(s: any) =>
+                                                    this.handleEmojiShortCodeChange(s.target.value, index)


To pass data to linkEvent, do:

onClick = {linkEvent( {some_data_here}, this.handleBleh )}

dessalines · 2023-02-21T14:37:48Z

src/shared/components/home/emojis-form.tsx

+                            </tr>
+                        </thead>
+                        <tbody>
+                            {this.state.customEmojis.slice((this.state.page - 1) * this.itemsPerPage, ((this.state.page - 1) * this.itemsPerPage) + this.itemsPerPage)


Leave complicated front-end paging logic out of this, if we need to later, we can add it in the back end.

Regarding paging: All emojis are already loaded on frontend, no need to pull pages from server. We are paging on admin settings because showing all emojis at once (especially when you get to the thousands) causes page to slow to a crawl and become unusable.

If we're really getting to the level of crashing the front end because there are too many emojis, then we really need to take them out of GetSite into a paged emoji fetcher.

Front end hacks like this have to be replicated on all lemmy front ends, which means we're not structuring things correctly. I'm okay with merging this as is, but please add an issue to the Lemmy repo for breaking emojis into its own paged endpoint.

I would consider adding a hardcoded emoji limit in the backend then. If you hit the limit, you just need to remove an existing emoji before adding a new one.

All users need all of the emojis available; when they open the emoji-picker, we need to know what to put in there all at once, not paged. Because of this, its kind of pointless to also have paged endpoints for emojis. I personally consider front-end paging pretty trivial; its mainly necessary once you get to the number of emojis that sites like hexbear have, which ends up with >2000 elements. Rendering all those at once is unfortunately a weakness of html and will cause page to get unresponsive.

Adding a hard coded limit would be unfortunate, as hexbear would be way over the limit of what is responsive on that page. Seems unnecessary, since the obvious way to work with it is just paginating on the front end. Alternate frontends can go without paginating, as long as they understand that it will get slower as more emojis are added.

dessalines · 2023-02-21T14:45:37Z

src/shared/components/home/emojis-form.tsx

+        this.setState({ page: page });
+    }
+
+    handleEmojiClick = (e: any) => {


No closures, functions only please.

src/shared/components/home/emojis-form.tsx

makotech222 · 2023-02-21T18:05:56Z

@dessalines Okay I fixed the comments. I updated the table to be more like communities. I do still have the last column with a set width, because the table doesn't really allow two elements in the same column without wrapping it, causing the row width the expand
desktop:

mobile:

dessalines

Once the back-end gets merged, I'll test this again, and merge it if everything works correctly.

dessalines · 2023-03-15T18:56:43Z

src/shared/components/home/emojis-form.tsx

+                            </tr>
+                        </thead>
+                        <tbody>
+                            {this.state.customEmojis.slice((this.state.page - 1) * this.itemsPerPage, ((this.state.page - 1) * this.itemsPerPage) + this.itemsPerPage)


If we're really getting to the level of crashing the front end because there are too many emojis, then we really need to take them out of GetSite into a paged emoji fetcher.

Front end hacks like this have to be replicated on all lemmy front ends, which means we're not structuring things correctly. I'm okay with merging this as is, but please add an issue to the Lemmy repo for breaking emojis into its own paged endpoint.

makotech222 · 2023-03-25T23:45:40Z

@dessalines Hey, any open requests for changes here? Or just need to test?

dessalines · 2023-03-27T16:51:04Z

Okay I deployed the new lemmy-js-client, and merged this fixing a few conflicts.

makotech222 force-pushed the custom-emojis branch 2 times, most recently from acef14f to 7e08763 Compare December 12, 2022 23:02

makotech222 force-pushed the custom-emojis branch 3 times, most recently from 7aac252 to ec86651 Compare December 15, 2022 18:48

makotech222 changed the title ~~[Draft] Add Custom Emojis Support~~ Add Custom Emojis Support Dec 15, 2022

makotech222 marked this pull request as ready for review December 15, 2022 19:11

makotech222 requested a review from dessalines as a code owner December 15, 2022 19:11

makotech222 mentioned this pull request Dec 15, 2022

Add Custom Emojis Support LemmyNet/lemmy#2616

Merged

makotech222 force-pushed the custom-emojis branch 3 times, most recently from 105e39e to 390b44e Compare December 30, 2022 16:23

makotech222 force-pushed the custom-emojis branch 2 times, most recently from 6a25461 to a4212dd Compare January 7, 2023 23:01

makotech222 force-pushed the custom-emojis branch from 68708f0 to e8f57bc Compare February 1, 2023 01:04

dessalines reviewed Feb 6, 2023

View reviewed changes

makotech222 force-pushed the custom-emojis branch 2 times, most recently from 00cdbd8 to 000e1f6 Compare February 12, 2023 04:05

makotech222 force-pushed the custom-emojis branch from 4d0d280 to 0d64ebc Compare February 18, 2023 16:18

dessalines reviewed Feb 21, 2023

View reviewed changes

makotech222 force-pushed the custom-emojis branch from cca7d16 to fc85823 Compare February 25, 2023 15:19

makotech222 added 5 commits February 26, 2023 11:22

Add Custom Emoji Support

1f15ea5

fix casing

c22eafa

Fix loading emojis

d7f4d2c

Fix improper use of map

f1147f5

Fix table layout on mobile

96827a0

makotech222 force-pushed the custom-emojis branch from c6205fe to 96827a0 Compare February 26, 2023 17:22

Merge branch 'main' into custom-emojis

4b6851d

dessalines reviewed Mar 15, 2023

View reviewed changes

dessalines merged commit 4499579 into LemmyNet:main Mar 27, 2023



		let custom_emojis: any[] = [];
		let custom_emojis_lookup: { [k: string]: CustomEmojiView } = {};

Add Custom Emojis Support #877

Add Custom Emojis Support #877

Conversation

makotech222 commented Dec 11, 2022 • edited Loading

Nutomic commented Dec 12, 2022

dessalines commented Dec 14, 2022

makotech222 commented Dec 14, 2022

makotech222 commented Jan 8, 2023

dessalines commented Feb 6, 2023

makotech222 commented Feb 6, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dessalines commented Feb 6, 2023

makotech222 commented Feb 11, 2023

makotech222 commented Feb 12, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

makotech222 commented Feb 21, 2023

dessalines left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

makotech222 commented Mar 25, 2023

dessalines commented Mar 27, 2023

makotech222 commented Dec 11, 2022 •

edited

Loading