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
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…
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?
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.
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.
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…
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…
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…
Removing Babble makes it work fine. However, as @ehemsley has pointed out:
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.
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.
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.
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.
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.
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.
@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
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.
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.