search

andrewmcgivery / obsidian-soundscapes

A plugin for Obsidian.MD that adds a music/ambiance player to the status bar to play Lofi beats, nature sounds, ambiance, relaxing music, and more.
MIT License
46 stars 7 forks source link

[Bug] Using Obsidian-Soundscapes with obsidian-statusbar-organizer completely breaks the status bar #21

Closed ThomasEricB closed 4 months ago

ThomasEricB commented 4 months ago

Basically the title. Using Obsidian-Soundscapes with obsidian-statusbar-organizer completely breaks the status bar. It starts spawning multiple instances of Soundscapes and it completely breaks the plugin.

Obsidian_ICa72BMmmN

Obsidian_3EaPCR5v0x

ThomasEricB commented 4 months ago

I also made this an issue at https://github.com/Opisek/obsidian-statusbar-organizer/issues/2

Opisek commented 4 months ago

Here's what happens:

It looks to me like your plugin logic is strictly embedded in the status bar element itself. E.g. The music completely stops playing when the element is gone, so it's not just a visualization.

Now, when my plugin adds your music player back, it reacts as if it had just been initialized, so it adds all the HTML components once more. The issue is, that the components never went anywhere, so we've now got duplicates.

I did find a way I could potentially fix this issue, but it would require a significant part of my code to be rewritten from scratch, so I'd like to avoid that for now.

Would it be possible to separate your plugin's logic from its UI in the status bar instead, so the playback logic is not embedded in the status bar element? @andrewmcgivery

It's fine if it's not viable. In that case I will focus on making my plugin slightly less invasive by using CSS order property instead of rearranging the divs. It's a better approach anyway in my opinion, but you might have to wait up to 3 weeks for me to implement it. I'll keep you posted. @ThomasEricB

Opisek commented 4 months ago

Good news @ThomasEricB

Turns out my time estimate to make the two plugins work together on my side was way off. I've resolved the issue here. Just update my plugin in Obsidian to 1.3.0.

As such, I've handled the compatibility on my side, so the issue can be closed.

The only annoyance is that the element preview for this plugin doesn't work, as naively copying innerHTML doesn't reflect the actual state.

andrewmcgivery commented 4 months ago

Thank you @Opisek for jumping on this quickly!

Out of curiosity, what ended up being the fix? Isn't jumping out to me in your PR what specifically fixed the compatibility. :)

EDIT: Nevermind! Just saw your message and the part of the PR. Thank you again!

ThomasEricB commented 4 months ago

Thanks to @Opisek and @andrewmcgivery for being really responsive on this. You guys rock ❤️