Iterating over controllers with same identifier


#1

Hi there!

I’ve read through this discussion and looked at the github issue it references and the source code of getControllerForElementAndIdentifier.

Here’s my situation:

I have 20 mp3s on a page. Right now that means I have 20 instances of <div class="mp3" data-controller="mp3"> on my page and they each have a play target and a seekBar target and so on…

The task: When I click play on an mp3, I want all the other mp3s to be told to pause.

Solutions I’ve looked at:

  1. Iterating over my mp3 controllers. I love that this.application.controllers is a thing! I wonder if we are missing something fancy to match the target/targets sugar like this.application.mp3Controllers. For now, I can manually cull them with .find. Related: this.application.controllers.indexOf(this) lets you identify the current controller while iterating, but it seems like there is potential to be able to do this.application.mp3Controllers.indexOf(this) + 1

  2. Iterating over the DOM and calling getControllerForElementAndIdentifier. The way Stimulus is written seems to suggest this is preferable to the above, but it feels a bit clunky.

  3. Make my mp3s targets instead of controllers. This way I could do something like this.pauseTargets.forEach... to tell them to all pause.

  4. Iterating over the DOM and triggering DOM events (which would in turn trigger action methods on controllers) @sam has mentioned this is a good option and on first glance seems solid for my use case: I want all other mp3s to be notified they need to stop playing. In this case, that would mean programmatically clicking a hidden element called “pause” which…feels a bit wrong, though. It might be nice if I could trigger a custom event called “pause”…is it possible to have an action descriptor with a custom event like pause->mp3#pause ?

As far as I can tell, this is the exact place where Stimulus wants to draw a line: Instead of expecting to manipulate and manage all state and behavior in js, we shouldn’t be afraid to walk through the DOM, fire events, add/remove class names, etc: Do whatever is most clear and direct. I’m all in agreement, just uncertain about this iteration case.

Curious about the perceived best practice, as well as if others are finding it a bit of a gray area.


#2

Hi!

This is a great breakdown of the problem at hand and the solutions you’ve explored. I’d like to offer a possible 5th solution that eliminates the issues with 1-4. Instead of iterating over all the controllers, you could track the current playing controller internally and ensure that only one is active:

// mp3_controller.js
import { Controller } from "stimulus"

let player

export default class extends Controller {
  disconnect() {
    this.pause()
  }

  play() {
    if (player) {
      player.pause()
    }
    player = this

    // …
  }

  pause() {
    if (player == this) {
      player = null
    }

    // …
  }
}

Would something like that work for you?


#3

:exploding_head:

Thanks Javan, really appreciate the explicit illustration.

Your 5th solution makes perfect sense to me. I blindly assumed iteration was required because that’s how we were doing things before. It makes a lot more sense to me to represent the player and store the active controller there.

Due to the sound library we are using, each mp3 is actually its own tag with some magic and controls around it, so doing something like this will be perfect:

  play() {
    if (player) {
      player.audio.pause()
    }
    player = this
    player.audio.play()
  }

Thanks for the hand!