search

sudara / alonetone

A free, open source, non-commercial home for musicians and their music
https://alonetone.com
MIT License
346 stars 89 forks source link

Update animation_heart and favorite counts when favoriting #1237

Closed Jberczel closed 11 months ago

Jberczel commented 12 months ago

When favoriting an asset, the favorite count does not update until you refresh the page.

In addition, if the asset/mp3 is displayed in multiple sections: "latest uploads", "recently favorited", "latest comments", the other instances of that asset do not get updated (heart svg does not update).

For the favorite counts, I added a turbo_stream update action to update count without having to refresh the page.

For the animation_heart state, I updated stimulus controller to broadcast to other instances of the same asset/mp3 to also toggle.

sudara commented 12 months ago

Hey this is cool! Thank you! Will pull and check it out on monday.

Jberczel commented 11 months ago

Hi @sudara - Were you able to take a look? Any issue or concern?

sudara commented 11 months ago

Hey @Jberczel! Yeah, I took a look before and after you pushed the last commit. I actually did have a Q, i'll ask it inline in the code...

Jberczel commented 11 months ago

@sudara - I didn't see any inlined comment.

sudara commented 11 months ago

So sorry! I forgot to hit submit on the "comment review"

sudara commented 11 months ago

I love the idea of having the counts update + the other hearts fire, but I think we should pick a more straightforward pattern to accomplish this vs. working around stimulus, either:

  1. the hearts count / toggle stream from the backend on update
  2. custom events update the relevant controllers (example)

I'll close this PR for now. If you want to take a crack at either of the above methods, feel free. If not, you've still motivated me to care about the subject, so, thank you!!

Jberczel commented 11 months ago

Sounds good. I'll take a look at custom events.

Jberczel commented 11 months ago

Hi @sudara - I refactored PR to use a more "stimulus-y" approach to get the other hearts to fire.

As you mentioned earlier, I first implemented using custom event.

But after digging into documentation for Stimulus 3.2, I discovered Stimulus Outlets API, which we can use for cross-controller communication as an alternative to dispatching custom events on controller elements.

See:

https://stimulus.hotwired.dev/reference/outlets

I've implemented with both approaches in two commits depending on which pattern you prefer.

Could you re-open PR?

sudara commented 11 months ago

Howdy!

Oooh outlets could be cool! If you have commits to both, would you mind pushing them to github so I could take a look. Sorry this wasn’t an insta-marge but it introduces a new pattern, so I’d like to get it right!

Whatever is easiest for you, you can point me to your branches on github or open new PR for them and we’ll merge one (doesn’t make sense to reopen this one)