gdpelican / retort

A reactions plugin for Discourse
MIT License
57 stars 39 forks source link

Retort-button just dims the site #28

Closed daath closed 7 years ago

daath commented 7 years ago

After a recent update, where I lost the retort-button, it's now back, but clicking it just dims the website (apart from some replies and buttons) - See screenshot. I tried clearing the cache etc, but it does the same...

Screenshot of it not working...

My users really love Retort, so I hope it gets fixed soon :)

mholt commented 7 years ago

Specifically:

TypeError: Cannot read property 'left' of undefined
    at s._positionPicker (application-87aba112f10f9884357019b139e31d86a190897efaa7717938d95fa42d51d5f3.js:sourcemap:20)
    at s.<anonymous> (application-87aba112f10f9884357019b139e31d86a190897efaa7717938d95fa42d51d5f3.js:sourcemap:20)
    at invokeWithOnError (_ember_jquery-a8dcbd325e04410f036f2a791d66d8316c48c5387acdd914de99a5dd6afb3cd3.js:12021)
    at a.flush (_ember_jquery-a8dcbd325e04410f036f2a791d66d8316c48c5387acdd914de99a5dd6afb3cd3.js:12080)
    at u.flush (_ember_jquery-a8dcbd325e04410f036f2a791d66d8316c48c5387acdd914de99a5dd6afb3cd3.js:12204)
    at p.end (_ember_jquery-a8dcbd325e04410f036f2a791d66d8316c48c5387acdd914de99a5dd6afb3cd3.js:12274)
    at _ember_jquery-a8dcbd325e04410f036f2a791d66d8316c48c5387acdd914de99a5dd6afb3cd3.js:12840
schungx commented 7 years ago

Only the last post puts up the emoji pane. All other posts that are not the last one will not have the emoji pane, just dimming the screen instead.

schungx commented 7 years ago

display: flex is added to the emoji-picker of the wrong post.

The correct dark background overlay is activated (i.e. the one associated with the relevant post got set to --active), but always the last emoji-picker is set to flex. This is a bug.

The correct behavior is to set display: flex on the emoji-picker of the relevant post.

schungx commented 7 years ago

File retort-picker.js.es6:

_loadCategoriesEmojis() {
    if (siteSettings.retort_limited_emoji_set) {
      const $picker = $('.emoji-picker')
      $picker.html("")
      siteSettings.retort_allowed_emojis.split('|').map((code) => {
        $picker.append(`<button type="button" title="${code}" class="emoji" />`)
        $(`button.emoji[title="${code}"]`).css("background-image", `url("${emojiUrlFor(code)}")`)
      })
      this._bindEmojiClick($picker);
    } else {
      this._super()
    }
  }

The line:

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

should pick up a whole array of emoji pickers (each one with class emoji-picker), and the line:

this._bindEmojiClick($picker);

probably needs to bind to only one picker (the one within the current post). However, since it binds to a list, I suspect that it is always binding to the last picker, which is the picker within the last post. And since that picker is hidden by the wrapper if it is not active, no picker ends up being shown.

Now, $picker = $('.emoji-picker') might just pick up the last DOM element with the class emoji-picker, in which case it would have produced the current behavior right away.

I am pretty sure this is a bug, and the line should probably be:

const $picker = $('.emoji-picker', this.rootDomNode)

or something to base the CSS search off the current root of the DOM tree to get that particular emoji picker under the current post.

Sophilautia commented 7 years ago

Really hope this gets updated soon! Our forum uses this plugin quite regularly, so it's a real shame to see it broken with the latest update.

mholt commented 7 years ago

Huh, it's been working for me lately. (I keep Discourse up to date.) Also noticed some nice new UI features.

daath commented 7 years ago

Aspirety, it was fixed already... Just update the retort plugin :)