Retort - a reaction-style plugin for Discourse


(Evan Hemsley) #147

It mostly works now, thanks for the fixes!

One issue is that sometimes clicking the retort button dims the site but doesn’t pop up the emoji picker.
After a ctrl-R refresh it works again but it’s a little frustrating.

Tested on Opera

EDIT: after some testing it looks like bringing up the reply window, closing it, and then trying the retort button again is what triggers the issue


(Stephen Chung) #148

So, obviously nobody seem to be having the same problem with on-screen formatting that I have:

Can this be a conflict with another plugin? The broken previous version was formatting the emoiji box correctly, just won’t show it. How it is shown but formatted obviously incorrectly.

Bummer is… nobody seems to be seeing this except poor me… :disappointed_relieved:


(James Kiesel) #149

It seems likely to be a caching issue to me. Have you tried clearing your cache and reloading? Can you confirm that it happens in multiple browsers?


(Stephen Chung) #150

Yes, I confirm that this happens in all browsers, with caching turned off.

A screenshot on FireFox, with the DOM tree in inspector.

Since nobody else seems to be encountering this, I suspect this might be a plugin conflict…

These are the plugins I have:

Name Version Enabled?
babble 2.0.4 Y
discourse-data-explorer 0.2 Y
discourse-details 0.4 Y
discourse-formatting-toolbar 2.3 Y
discourse-narrative-bot 0.0.1 Y
discourse-nginx-performance-report 0.1 Y
discourse-presence 1.0 Y
discourse-solved 0.1 Y
Spoiler Alert! 0.3 Y
discourse-translator 0.2.0 Y
discourse-voting 0.4 Y
discourse-whos-online 1.0 Y
docker_manager 0.1 Y
lazyYT 1.0.1 Y
poll 0.9 Y
retort 1.1.2 Y

(Stephen Chung) #151

If retort limited emoji set is OFF, then it pops up with the standard emoji picker (you know, the one that pops up when writing a reply).

If retort limited emoji set on ON, then it causes this problem, obviously because the wrapper is missing a few DOM panels inside and the buttons got spread directly by the flexbox.

As it happens, the code in retort-picker.js.es6 defers to the standard emoji picker if retort limited emoji set is OFF, which explains the current bahavior. However, if retort limited emoji set is ON, then somehow the standard emoji picker still gets shown, instead of retort emoji picker (which has all the relevant buttons created correctly). So, in other words, this._bindEmojiClick($picker); is not having the desired effect.

There are two emoji pickers on the page: the standard emoji picker, and the retort emoji picker. The both have the class emoji-picker. I am quite sure that the line const $picker = $('.emoji-picker') is not correct, since it will return both pickers (when you only want the retort emoji picker). As a result, both the retort emoji picker and the standard emoji picker got buttons created underneath, if retort limited emoji set on ON.

And the critical line this._bindEmojiClick($picker); binds the click to two pickers, which probably means the last one wins (i.e. the standard emoji picker, which is located below the retort emoji picker).

I am quite sure the correct code should be:

const $picker = $('.retort-picker.emoji-picker')

I suspect nobody here limits the emoji’s to a list? :thinking:


(James Kiesel) #152

Hey @schungx the best approach here is going to be to submit a PR to the repo with a fix :slight_smile:


(Stephen Chung) #153

Problem is:

  1. I don’t know Ruby and Rails
  2. I don’t know how the system works mostly
  3. I don’t know how to do a PR…

:sweat_smile:

Is there, sort of, a “dummies-way” in Github to package a small change as a PR without me having to clone the entire repository?

That’s because I have no way to test my proposed changes. Of course I can probably rebuild the Discourse with my own version of retort, but I am not quite sure how to do that since it involves hooking up to a local Git repository and things get complicated and confusing very quickly…

I am quite sure there is a way to just enter the container and modify the JS file to test things… any pointers into how to do that will be welcome. Also, an indication which file it gets turned into (so I can change the code and test to see if it works) will be nice.


(Joshua Rosenfeld) #154

Yes. Create a Github account, find the file in the repository on Github, and there will be an edit icon. As you don’t have write access, it will automatically fork the repo for you and create a branch. Edit the file right on the website, then push it to your repo. Github will them prompt you to submit a PR.


(Stephen Chung) #155

@jomaxro Ha great! “Dummies” proof! :+1:

@gdpelican I have submitted (my first ever) PR.

Still I need a (dummy-simple) way to modify an existing running container to test out changes without rebuilding the whole thing with a private repository…


(Joshua Rosenfeld) #156

You’d need a development instance for that. I don’t think there’s a “dummy-simple” setup for development…


(Stephen Chung) #157

Well, good news and bad news.

Good news is that forking the repository in Github and then branching, I can simply rebuild Discourse by pulling from my fork instead. Then I can test changes. Great!

Bad news is: my proposed changes didn’t work totally… So it is back to more debugging…:sweat_smile:


(Stephen Chung) #158

After a couple of (long) compiles:

  1. Retort conflicts with Babble. If Babble is installed, Retort will popup the Babble emoji picker instead and screw the whole thing up. This is quite strange since both plugins seem to come from the same author…

  2. Removing Babble makes it work fine. However, as @ehemsley has pointed out:

  1. This is because, in Discorse, the EmojiPicker class stores the current emoji picker in a GLOBAL variable called $picker. Multiple plugins now fight for the same global $picker variable. Since retort-picker is always loaded first, $picker is set to this picker (if Babble is not installed). Once we bring up the reply-post emoji picker, $picker got changed to something else. So after that, retort-picker no longer works because $picker is no longer pointing to it.

  2. The solution is, obviously, to insert the element on click, and remove it when closed. This ensures that only one emoji picker is ever present at any time, and $picker gets reset every time.

  3. In the long run, there really is no reason for Discourse’s EmojiPIcker class to store the current picker in a GLOBAL variable; it should easily be moved to an instance-based field so that multiple pickers (from different plugins) can co-exist without screwing each other up.

I’ll try to put out a PR when I figure out how to dynamically add/remove widgets.


(Stephen Chung) #159

Tthe trick is to call didInsertElement before show and call didDeleteElement after close. This should reset the $picker global variable, and doesn’t appear to have any ill effects so far.

WARNING: LARK’S VOMIT – This is a HACK.

I’ve submitted a new PR.

It doesn’t work together with discourse/babble though because it has the same problem with the emoji-picker. The problem is with discourse/babble, not with retort.


(Evan Hemsley) #160

Just tried your fix. Big improvement but there’s one lingering bug:

Open a reply window, click the emoji picker in the reply window, and then click the retort button on a post. The modal will show up empty and make the browser window unresponsive.


(Stephen Chung) #161

Yup, it is a hack only… The standard EmojiPicker in Discourse uses module-global variables to store the picker to popup. Any time the reply window is opened, it gets overwritten to point to a different emoji picker, throwing everything off.

I’ve opened an issue for this:

When the reply window is open, the variable points to the standard EmojiPicker. When you press the retort button, my fix forced-overwrite the variable to point to the RetortEmojiPicker. But if the reply window (and the standard EmojiPicker) is still open, obviously two emoji pickers together are going to make things very interesting… especially when there is only one module-global variable to point to one of them.

:sunglasses:


(Stephen Chung) #162

With changes in Discourse’s emoji picker in recent versions, this plugin now works again.

I have submitted a PR for some minor fixes to pick up the correct emoji picker when the reply window is open.


#163

@gdpelican Would it be possible to merge that pull request and release a new version? The plugin is quite awesome and I’d like to turn it back on for my community :slight_smile:


(Stephen Chung) #164

Just edit container/app.yaml, comment out the original line and put in:

git clone https://github.com/schungx/retort.git

That works for me.

Until the PR is merged into the official branch.

But I see that @gdpelican has already fixed the before_filter issue with a new commit a few hours ago. You can just try rebuilding to see if it works. AFAIK, the changes to this.$() are not in yet so there may be conflicts if you have both the retort up as well as a reply window.


(DiscourseMetrics.com) #165

It would be nice to be able to limit use of Retort to those users with trust level > X. I have a banned user that keeps creating new accounts and then spams posts he dislikes with lots of negative retorts. :confused:


(James North) #166

You can limit to one retort per user per post … but yeah would be good.

Seems like that thing from Red Dead Redemption online where people kept shooting your horse because they can’t shoot you in friendly mode.