jneilliii / OctoPrint-PlotlyTempGraph

23 stars 7 forks source link

Graph not updating with UI Customizer #23

Closed kfrancis closed 2 years ago

kfrancis commented 2 years ago

Love the plugin.

Since I installed UI Customizer, the graph doesn't update

jneilliii commented 2 years ago

Any errors in your developers tool's (F12) console tab? Don't know if @LazeMSS might know anything about this or not.

jneilliii commented 2 years ago

I just tested in my UI Customizer instance and the graph seems to be working. Will need more details.

LazeMSS commented 2 years ago

Also works on my development environments

Dak0r commented 2 years ago

I use Ui Customizer with the Flatly theme and everything seems to be fine on my setup

kfrancis commented 2 years ago

Hmm, I'll see what I can gather. I didn't see any errors, just some warnings that looks relevant (about temp).

kfrancis commented 2 years ago

Using theme: Discorded

Versions

Dev tools has messages like:

Did not bind view model TemperatureViewModel to target #temp since it does not exist
Did not bind view model TemperatureViewModel to target #temp_link since it does not exist

2021-10-14 13_07_51-Predator OctoPrint  OctoPrint  and 5 more pages - Personal - Microsoft​ Edge

kfrancis commented 2 years ago

I would be comfortable letting @jneilliii in via octo-everywhere if that would help debug, just message me.

kfrancis commented 2 years ago

I'll note also that it's just the updating that's not working. The graph does generate correctly and can be interacted with normally. It can be refreshed when I hit F5, but otherwise doesn't:

2021-10-14 13_12_01-Predator OctoPrint  OctoPrint  and 5 more pages - Personal - Microsoft​ Edge

jneilliii commented 2 years ago

Ah, this might be the temperature widget in UI Customizer interfering since my plugin replaces the default view model. I don't enable that in my instance. If you hide that in UI Customizer's settings does it work after refreshing the page?

jneilliii commented 2 years ago

@LazeMSS I think this could be relatively easy to fix using optional viewmodel dependency with this plugin's identifier, and redefining self.tempModel to use that parameter[index] if it exists. I'll do some testing when I get home tonight and submit a PR to UI Customizer if it works.

LazeMSS commented 2 years ago

@LazeMSS I think this could be relatively easy to fix using optional viewmodel dependency with this plugin's identifier, and redefining self.tempModel to use that parameter[index] if it exists. I'll do some testing when I get home tonight and submit a PR to UI Customizer if it works.

Awesome!

jneilliii commented 2 years ago

@kfrancis I couldn't message you via twitter. could you do a test for me and see if installing this version of the UICustomizer has any effect? OctoPrint settings > Plugin Manager > Get More > ...from URL.

https://github.com/jneilliii/OctoPrint-UICustomizer/archive/refs/heads/bugfix/plotlytempgraph.zip
jneilliii commented 2 years ago

I will say, even after enabling the temperature widget in UI Customizer the original version was still working for me, so this could potentially be a combination of multiple plugins causing this. I would be willing to connect to your instance to debug, just shoot me an email or reach out on the OctoPrint Discord server.

kfrancis commented 2 years ago

@kfrancis I couldn't message you via twitter. could you do a test for me and see if installing this version of the UICustomizer has any effect? OctoPrint settings > Plugin Manager > Get More > ...from URL.

https://github.com/jneilliii/OctoPrint-UICustomizer/archive/refs/heads/bugfix/plotlytempgraph.zip

Interestingly, that didn't install this plugin but the UI customizer was reinstalled. After doing that, it just seems to work now no matter if I have the default temp widget enabled or not.

jneilliii commented 2 years ago

Yeah, that is my fork of ui customizer with potential fix for the issue, although I'm not sure it was an issue to begin with. Would definitely resolve the error you posted I think though. @LazeMSS do you see any potential issues with that branch?

LazeMSS commented 2 years ago

No basically what you are doing is replacing the native temp model with plotlytempgraphViewModel if present - it looks nice and simple :)

LazeMSS commented 2 years ago

Do a PR

jneilliii commented 2 years ago

PR submitted.