willasm / obsidian-open-weather

Obsidian plugin for OpenWeather API
MIT License
47 stars 3 forks source link

Multiple dynamic divs in reading and source view lead to no-render #30

Closed Accelsnow closed 1 week ago

Accelsnow commented 2 months ago

First, many thanks to the great plugin! I just noticed that sometimes the dynamic div does not get rendered so I investigated a bit.

When there are multiple dynamic divs with the same class, the weather div will not get updated. I took a look at the plugin source code and found the culprit in main.js. Looks like the update only happens when there is one and only one div with the dynamic div class name:

if (document.getElementsByClassName("weather_current_4").length === 1) {
<do update...>
}

Some simple console log shows that there are multiple divs of the same className 'weather_current4', although I have one and only one_ openweather div in my note:

image

Some further digging into the Elements shows that the two divs appear under markdown-source-view and markdown-reading-view.

image

I was able to circumvent this problem by:

for (let i = 0; i < document.getElementsByClassName("weather_current_4").length; i++) {
    const divEl = document.getElementsByClassName("weather_current_4")[i];
<subsequent logic...>
}

Not sure if this is acceptable :)

willasm commented 2 months ago

Nice catch. I never anticipated someone using the div more than once in a document and as I never had a need to do that it simply never occurred to me. I will have a look at it when I get home from my trip. I suspect I will need to make this change for the "weather_current_3" code as well.

Thanks for spotting this and bringing it to my attention, William.

Accelsnow commented 2 months ago

Nice catch. I never anticipated someone using the div more than once in a document and as I never had a need to do that it simply never occurred to me. I will have a look at it when I get home from my trip. I suspect I will need to make this change for the "weather_current_3" code as well.

Thanks for spotting this and bringing it to my attention, William.

I don't know if it's some of my plugins, but it also appears that when the page switches between source mode and read mode, the div automatically gets replicated (one under markdown_reading_view and one under markdown_source_view. So after switching, the weather div may also become hidden (as there are 2 divs now). This is shown in my screenshot above.

And yes, from my installed main.js, I simply replaced all 4 div classes weather_current_1234. I've been using it for a day and haven't really spotted any performance or other issues.

willasm commented 2 months ago

I finally have some free time to work on this. I'm very confused by this, I am not seeing this under the class "markdown-source-view". I only see it under class "markdown-reading-view" and it doesn't matter how many times I switch views?

Could you please post a copy of your template and a copy of a resulting note (Remove any personal data first of course). I need to be able to replicate this to find out the actual cause is for what you are seeing.

I did find one bug though. The default settings for weather string 3 and 4 should be displayed on 4 lines but are being displayed on one only. That is easy to fix though, just replace all the <br> with 2 spaces + enter (new line) in the settings. That must have broke in one of the v1.6 updates.

Thanks, William.

Accelsnow commented 2 months ago

It's weird. I just tried the bug in both my vault and a fresh vault with just OpenWeather plugin. Both of them appear to work fine now. The source view indeed treats the dynamic div as a list of spans (instead of an actual div like I posted in the screenshot above). I am not sure how I observed the double div shown in the screenshot. Maybe one of my 65 plugins was bugged and then got fixed?

Sorry for the confusion!

willasm commented 2 months ago

Thanks for letting me know. That is what I am seeing as well, they are showing as a bunch of spans. The v1.6 update moved a lot of the UI elements which is likely causing issues for some plugins that are using CSS. I still need to fix the snippet I use for the file explorer, that is completely broken as everything in that panel got moved around. I'll leave this open for now in case someone else has a similar issue or if it happens again to yourself.

Thanks, William.

willasm commented 1 week ago

I just want to inform everyone that this plugin in being discontinued. Open Weather has discontinued the API 2.5 on which this plugin is based. I have however created a new plugin using the Visual Crossing Weather API and it follows the same basic approach to representing the weather as this plugin did so it should be relatively easy to move over to the new plugin. While it is still in development it is almost 100% functional. The documentation however is almost non existent but the list of available macros is complete and is in a nicely formatted table for better viewing. I hope to have it published soon as it is very close to being completed. If you want to try it before it is released you can find it on Github here.

Thank you, William McKeever