Emoji Support [WIP]
Review Request #7708 — Created Oct. 16, 2015 and discarded
Information | |
---|---|
phngo | |
Review Board | |
new-branch | |
Reviewers | |
reviewboard, students | |
This project would add this support to any text field in Review Board, by listening for when the user types ":" (followed by two letters) at the beginning of a line or after a space, and presenting a list of possible Emoji.
The plan:
1) Find a library like Twemoji that will render unicode emoji characters & emoji shortnames (e.x.
) to an image.
2) Get emoji shortnames to render.
3) Work on shortname autocomplete.
4) Work on emoji picker UI.
4) Handle MySQL errors with Native Unicode EmojisTasks:
- DONE -- Get emoji shortnames to render
- DONE -- Have markdown ON/OFF decided whether the emojis should be rendered or not
- DONE -- Create autocomplete
- IN PROGRESS -- Create Emoji Picker UI
Changes/Commits:
- 10/16/15 -- Added Emojione.min.js lib. Working on getting emoji shortname (e.x.
) to render after typing into text editor and pressing "OK". Running into issues where after making a comment with emoji shortname, emojis do not render when first loading a page. Also, another issue -- emojis will render when "Enable Markdown" is selected or deselected (options.richText is true).
- 10/23/15 -- No changes to code. Added more detail to description. Spent time researching autocomplete and codemirror.
- 11/1/15 -- Emojis will render correctly. Initially had unicode emojis saving into database as shortnames working, however, I realized that the code for this was written into jquery.gravy.inlineEditor.js which is in djiblets.
- 11/8/15 -- Completed emoji support with markdown ON/OFF. Now, Emojis will render only if markdown is enabled (checked). Emoji shortnames & Unicode emojis will revert from rendered emoji images if markdown is disabled.
- 11/15/15 -- Fixed style issues. Working on emoji autocomplete and UI.
- 11/19/15 -- Implemented Emoji Autocomplete & UI. Added library jquery.textcomplete.min.js. Several known bugs exist.
- 11/29/15 -- Refactored setEmojiAutoComplete and fixed issues. Still working on solving bugs.
- 12/1/15 -- Fixed Emoji Autocomplete bugs. Also changed stylesheet to appropriately size emojis.
- 12/5/15 -- Added Emoji Button to Markdown Enabled field.
Description | From | Last Updated |
---|---|---|
if statement should have whitespace before the bracket: if (condition). |
JW jwu | |
I believe when you have an if statement on multiple lines, you should line the conditions up: if (condition1 || β¦ |
JW jwu | |
This too should be lined up similarily to the if statement conditions: variable = someLongFunction(argument1, argument2, argument3); |
JW jwu | |
Same issue as above (not lined up properly). |
JW jwu | |
Whitespace after the if statement. |
JW jwu | |
We can refactor this to: newText = RB.LinkifyUtils.linkifyText(node.textContent, bugTrackerURL); if (el.children.length || (el.children.length === 0 && el.nodeName === 'P')) { β¦ |
|
|
$el.children().length |
|
|
The code that was there before is faster, because it doesn't have to loop through all children and wrap them β¦ |
|
|
if ($el.children().length) (undefined and 0 are both falsy). |
|
|
Same here. |
|
|
This isn't from your change. |
|
|
This looks to be duplicated. We don't want to duplicate code, so it's important that complex logic like this be β¦ |
|
|
can we make the node.textContent in next line? |
XU xutong |
-
I know this is still early, but I'd like to see a description of how you'll be tackling this. There are some real gotchas with this, and it's crucial we make sure we have a solid design before you do much development.
Change Summary:
Made a better description
Description: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Groups: |
|
Change Summary:
Edited description to give more detail on project. Not much changes to code.
Description: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commit: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 2 (+6 -1) |

-
Tool: Pyflakes Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/utils/textUtils.js reviewboard/static/lib/js/emojione.min.js Tool: PEP8 Style Checker Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/utils/textUtils.js reviewboard/static/lib/js/emojione.min.js
Change Summary:
- 11/1/15 -- Emojis will render correctly. Initially had unicode emojis saving into database as shortnames working, however, I realized that the code for this was written into jquery.gravy.inlineEditor.js which is in djiblets.
Description: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Branch: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Commit: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 3 (+7 -2) |

-
Tool: PEP8 Style Checker Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/utils/linkifyUtils.js reviewboard/static/rb/js/utils/textUtils.js reviewboard/static/lib/js/emojione.min.js Tool: Pyflakes Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/utils/linkifyUtils.js reviewboard/static/rb/js/utils/textUtils.js reviewboard/static/lib/js/emojione.min.js
Change Summary:
- Edited description. Removed converting unicode to shortnames.
- Completed emoji support with markdown ON/OFF. Now, Emojis will render only if markdown is enabled (checked). Emoji shortnames & Unicode emojis will revert from rendered emoji images if markdown is disabled.
Description: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commit: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 4 (+26 -3) |

-
Tool: PEP8 Style Checker Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/utils/linkifyUtils.js reviewboard/static/rb/js/utils/textUtils.js reviewboard/static/lib/js/emojione.min.js Tool: Pyflakes Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/utils/linkifyUtils.js reviewboard/static/rb/js/utils/textUtils.js reviewboard/static/lib/js/emojione.min.js
-
-
reviewboard/static/rb/js/utils/linkifyUtils.js (Diff revisions 3 - 4) if
statement should have whitespace before the bracket:
if (condition)
. -
reviewboard/static/rb/js/utils/linkifyUtils.js (Diff revisions 3 - 4) I believe when you have an if statement on multiple lines, you should line the conditions up:
if (condition1 || condition2) { }
Here you have an extra spcae which misaligns the two conditions. If you fix the whitespace issue on the previous line, then the conditions will line up.
-
reviewboard/static/rb/js/utils/linkifyUtils.js (Diff revisions 3 - 4) This too should be lined up similarily to the if statement conditions:
variable = someLongFunction(argument1, argument2, argument3);
-
reviewboard/static/rb/js/utils/linkifyUtils.js (Diff revisions 3 - 4) Same issue as above (not lined up properly).
-
reviewboard/static/rb/js/utils/textUtils.js (Diff revisions 3 - 4) Whitespace after the
if
statement.
Change Summary:
Fixed style issues
Description: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commit: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 5 (+28 -3) |

-
Tool: Pyflakes Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/utils/linkifyUtils.js reviewboard/static/rb/js/utils/textUtils.js reviewboard/static/lib/js/emojione.min.js Tool: PEP8 Style Checker Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/utils/linkifyUtils.js reviewboard/static/rb/js/utils/textUtils.js reviewboard/static/lib/js/emojione.min.js
-
-
reviewboard/static/rb/js/utils/linkifyUtils.js (Diff revision 5) We can refactor this to:
newText = RB.LinkifyUtils.linkifyText(node.textContent, bugTrackerURL); if (el.children.length || (el.children.length === 0 && el.nodeName === 'P')) { newText = emojione.toImage(newText); }
-
-
reviewboard/static/rb/js/utils/textUtils.js (Diff revision 5) if ($el.children().length)
(
undefined
and 0 are both falsy).
Change Summary:
- 11/19/15 -- Implemented Emoji Autocomplete & UI. Added library jquery.textcomplete.min.js. Several known bugs exist.
Description: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commit: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 6 (+248 -7) |

-
Tool: Pyflakes Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/lib/js/jquery.textcomplete.min.js reviewboard/static/rb/js/utils/textUtils.js reviewboard/static/lib/css/emoji_dropdown.css reviewboard/static/rb/js/utils/linkifyUtils.js reviewboard/static/rb/js/ui/views/textEditorView.js reviewboard/static/lib/js/emoji_strategy.json reviewboard/static/lib/js/emojione.min.js Tool: PEP8 Style Checker Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/lib/js/jquery.textcomplete.min.js reviewboard/static/rb/js/utils/textUtils.js reviewboard/static/lib/css/emoji_dropdown.css reviewboard/static/rb/js/utils/linkifyUtils.js reviewboard/static/rb/js/ui/views/textEditorView.js reviewboard/static/lib/js/emoji_strategy.json reviewboard/static/lib/js/emojione.min.js
-
-
reviewboard/static/rb/js/utils/linkifyUtils.js (Diff revision 6) can we make the node.textContent in next line?
-
-
reviewboard/static/rb/js/utils/textUtils.js (Diff revisions 5 - 6) The code that was there before is faster, because it doesn't have to loop through all children and wrap them in jQuery objects. Since we're only checking if there are children, there's no need to do this extra work.
-
-
reviewboard/static/rb/js/ui/views/textEditorView.js (Diff revision 6) This looks to be duplicated. We don't want to duplicate code, so it's important that complex logic like this be only in one place.
Change Summary:
- 11/29/15 -- Refactored setEmojiAutoComplete and fixed issues. Still working on solving bugs.
Description: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commit: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 7 (+202 -7) |

-
Tool: PEP8 Style Checker Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/lib/js/jquery.textcomplete.min.js reviewboard/static/rb/js/utils/textUtils.js reviewboard/static/lib/css/emoji_dropdown.css reviewboard/static/rb/js/utils/linkifyUtils.js reviewboard/static/rb/js/ui/views/textEditorView.js reviewboard/static/lib/js/emoji_strategy.json reviewboard/static/lib/js/emojione.min.js Tool: Pyflakes Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/lib/js/jquery.textcomplete.min.js reviewboard/static/rb/js/utils/textUtils.js reviewboard/static/lib/css/emoji_dropdown.css reviewboard/static/rb/js/utils/linkifyUtils.js reviewboard/static/rb/js/ui/views/textEditorView.js reviewboard/static/lib/js/emoji_strategy.json reviewboard/static/lib/js/emojione.min.js
Change Summary:
- 12/1/15 -- Fixed Emoji Autocomplete bugs. Also changed stylesheet to appropriately size emojis.
Description: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commit: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 8 (+224 -6) |

-
Tool: Pyflakes Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/lib/js/jquery.textcomplete.min.js reviewboard/static/rb/js/utils/textUtils.js reviewboard/static/lib/css/emoji_dropdown.css reviewboard/static/rb/js/utils/linkifyUtils.js reviewboard/static/rb/js/ui/views/textEditorView.js reviewboard/static/lib/js/emoji_strategy.json reviewboard/static/lib/js/emojione.min.js Tool: PEP8 Style Checker Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/lib/js/jquery.textcomplete.min.js reviewboard/static/rb/js/utils/textUtils.js reviewboard/static/lib/css/emoji_dropdown.css reviewboard/static/rb/js/utils/linkifyUtils.js reviewboard/static/rb/js/ui/views/textEditorView.js reviewboard/static/lib/js/emoji_strategy.json reviewboard/static/lib/js/emojione.min.js
-
-
reviewboard/static/rb/js/ui/views/textEditorView.js (Diff revisions 7 - 8) I'm not sure if .one is a valid function, but according to my initial pull, this is how it was initially. Somewhere in a past revision, I thought I might've accidently pressed 'e' so I removed it. As a result, this was causing a bug where the 'markdown enabled' checkbox to duplicate after making a change to a comment or description. Now that I put it back, no bugs :)
Change Summary:
- 12/5/15 -- Added Emoji Button to Markdown Enabled field.
Description: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commit: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 9 (+241 -6) |

-
Tool: Pyflakes Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/lib/js/jquery.textcomplete.min.js reviewboard/static/rb/js/utils/textUtils.js reviewboard/static/lib/css/emoji_dropdown.css reviewboard/static/rb/js/utils/linkifyUtils.js reviewboard/static/rb/js/ui/views/textEditorView.js reviewboard/static/lib/js/emoji_strategy.json reviewboard/static/lib/js/emojione.min.js Tool: PEP8 Style Checker Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/lib/js/jquery.textcomplete.min.js reviewboard/static/rb/js/utils/textUtils.js reviewboard/static/lib/css/emoji_dropdown.css reviewboard/static/rb/js/utils/linkifyUtils.js reviewboard/static/rb/js/ui/views/textEditorView.js reviewboard/static/lib/js/emoji_strategy.json reviewboard/static/lib/js/emojione.min.js