[NEW] Add pause and reset button when adding custom sound #13615
[NEW] Add pause and reset button when adding custom sound #13615rodrigok merged 3 commits intoRocketChat:developfrom
Conversation
knrt10
force-pushed
the
issue13614
branch
3 times, most recently
from
fb0fe9c to
812b420
Compare
knrt10
changed the title
|
@tassoevan would you please review |
| if ($audio && $audio[0] && $audio[0].play) { | ||
| $audio[0].play(); | ||
| } | ||
| }, 'click .icon-pause-circled'(e) { |
There was a problem hiding this comment.
| }, 'click .icon-pause-circled'(e) { | |
| }, | |
| 'click .icon-pause-circled'(e) { |
| if ($audio && $audio[0] && $audio[0].pause) { | ||
| $audio[0].pause(); | ||
| } | ||
| }, 'click .icon-reset-circled'(e) { |
There was a problem hiding this comment.
| }, 'click .icon-reset-circled'(e) { | |
| }, | |
| 'click .icon-reset-circled'(e) { |
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| const $audio = $(`audio#${ this._id }`); | ||
| console.log($audio); |
There was a problem hiding this comment.
| console.log($audio); |
| }, 'click .icon-pause-circled'(e) { | ||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| const $audio = $(`audio#${ this._id }`); |
There was a problem hiding this comment.
You must avoid jQuery dependency replacing $(`audio#{ this._id }`) with instance.find(`audio#{ this._id }`) (where instance is the second argument passed to this function).
There was a problem hiding this comment.
@tassoevan I think instance.find() is null here
There was a problem hiding this comment.
Ops, you're right, these <audio> tags are inserted somewhere else. Since they all have an ID, document.getElementById(this._id) will fit better.
tassoevan
added
type: bug
area: ui
|
@tassoevan updated the PR |
tassoevan
left a comment
tassoevan
left a comment
There was a problem hiding this comment.
Everything looks fine. As a final suggestion, please rename $audio as audio because it looks like Hungarian notation for jQuery objects.
|
Updated @tassoevan |
|
@tassoevan @rodrigok I dont think we need seperate buttons for play/pause. While playing any media file, we have a single button for play/pause which conditionally changes. Also multiple audio can be played here too which is a bug. I have made a PR for this. Would you please review. |
5 participants
Closes #13614