Conversation
Owner
jneilliii
commented
Aug 31, 2025
jneilliii
commented
- update plotly.js to version 3.1.0
- add visual indication of processing on Update button
- add theme support to graphs, resolves Theme Support #185
- split graphing into separate subplots, resolves Use separate scale/graph for power/cost #203
- add is_api_protected callback for newer OctoPrint versions, resolves Seems, plugin will stop working soon of not adapted to this change of octoprint 1.11.2 (is_api_protected) #211
…nto feature/split_graphs
add them support to graphs, #185
add visual indication of processing on Update button
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR bumps the plugin version to 1.1.5 and implements several key enhancements to the Tasmota plugin's graphing functionality.
- Updates plotly.js to version 3.1.0 for improved graph rendering
- Adds visual feedback during API requests with a spinning indicator on the Update button
- Implements theme support for graphs to match OctoPrint's UI themes
- Restructures graphing to use separate subplots for different data types
- Adds API protection callback for compatibility with newer OctoPrint versions
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| setup.py | Version bump from 1.1.4 to 1.1.5 |
| tasmota_tab.jinja2 | Adds spinner icon and processing state to Update button |
| tasmota.js | Major refactor of graph layout with theme support and subplot organization |
| init.py | Adds is_api_protected method for OctoPrint compatibility |
Comment on lines
+385
to
+386
| }).fail(function(){ | ||
| self.processing_api_request(false); |
There was a problem hiding this comment.
The fail callback should handle errors more gracefully. Consider logging the error or providing user feedback about the failed request.
Suggested change
| }).fail(function(){ | |
| self.processing_api_request(false); | |
| }).fail(function(jqXHR, textStatus, errorThrown){ | |
| self.processing_api_request(false); | |
| console.error("Failed to fetch energy data:", textStatus, errorThrown, jqXHR); | |
| alert("Failed to fetch energy data. Please check your connection or try again later."); |
| traces.push(trace_cost); | ||
| break; | ||
| default: | ||
| console.log('unknown energy data column ' + data.energy_data[i]) |
There was a problem hiding this comment.
This console.log statement will output the entire array instead of the column index. It should be console.log('unknown energy data column ' + j) to show the problematic column index.
| trace['yaxis'] = (data.energy_data.length > 0) ? 'y6' : 'y2'; | ||
| break; | ||
| default: | ||
| console.log('unknown sensor data column ' + data.sensor_data[i]) |
There was a problem hiding this comment.
This console.log statement will output the entire array instead of the column index. It should be console.log('unknown sensor data column ' + j) to show the problematic column index.
Suggested change
| console.log('unknown sensor data column ' + data.sensor_data[i]) | |
| console.log('unknown sensor data column ' + j) |
Comment on lines
+237
to
+240
| var inherited_bg_color = self.getInheritedBackgroundColor(document.getElementById('tab_plugin_tasmota')); | ||
| var background_color = (inherited_bg_color == 'rgba(0, 0, 0, 0)') ? '#FFFFFF' : inherited_bg_color; | ||
| var color_val = $('#tab_plugin_tasmota').css('color'); | ||
| var foreground_color = (!color_val || color_val === 'inherit' || color_val === 'transparent') ? '#000000' : color_val; |
There was a problem hiding this comment.
[nitpick] The theme color detection logic is duplicated and could be extracted into a separate function for better maintainability and reusability.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.