Emoji Support [WIP]

Review Request #7708 — Created Oct. 16, 2015 and discarded

Information

Review Board
new-branch

Reviewers

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 Emojis

Tasks:

  • 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')) { …

brenniebrennie

$el.children().length

brenniebrennie

The code that was there before is faster, because it doesn't have to loop through all children and wrap them …

chipx86chipx86

if ($el.children().length) (undefined and 0 are both falsy).

brenniebrennie

Same here.

chipx86chipx86

This isn't from your change.

brenniebrennie

This looks to be duplicated. We don't want to duplicate code, so it's important that complex logic like this be …

chipx86chipx86

can we make the node.textContent in next line?

XU xutong
reviewbot
  1. 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
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/static/rb/js/utils/textUtils.js
        reviewboard/static/lib/js/emojione.min.js
    
    
  2. 
      
chipx86
  1. 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.

  2. 
      
PH
PH
reviewbot
  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
    
    
  2. 
      
PH
reviewbot
  1. 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
    
    
  2. 
      
PH
reviewbot
  1. 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
    
    
  2. 
      
JW
  1. 
      
  2. reviewboard/static/rb/js/utils/linkifyUtils.js (Diff revisions 3 - 4)
     
     

    if statement should have whitespace before the bracket:
    if (condition).

  3. 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.

  4. 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);
    
  5. reviewboard/static/rb/js/utils/linkifyUtils.js (Diff revisions 3 - 4)
     
     
     

    Same issue as above (not lined up properly).

  6. reviewboard/static/rb/js/utils/textUtils.js (Diff revisions 3 - 4)
     
     
     

    Whitespace after the if statement.

  7. 
      
PH
reviewbot
  1. 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
    
    
  2. 
      
brennie
  1. 
      
  2. 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);
    }
    
  3. $el.children().length

  4. if ($el.children().length)

    (undefined and 0 are both falsy).

  5. 
      
PH
reviewbot
  1. 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
    
    
  2. 
      
brennie
  1. 
      
  2. reviewboard/static/rb/js/ui/views/textEditorView.js (Diff revision 6)
     
     
     
     
     
     

    This isn't from your change.

  3. 
      
XU
  1. 
      
  2. can we make the node.textContent in next line?

  3. 
      
chipx86
  1. 
      
  2. 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.

  3. reviewboard/static/rb/js/utils/textUtils.js (Diff revisions 5 - 6)
     
     

    Same here.

  4. 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.

  5. 
      
PH
reviewbot
  1. 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
    
    
  2. 
      
PH
reviewbot
  1. 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
    
    
  2. 
      
PH
  1. 
      
  2. 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 :)

  3. 
      
PH
PH
reviewbot
  1. 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
    
    
  2. 
      
david
Review request changed

Status: Discarded

Loading...