-
-
Notifications
You must be signed in to change notification settings - Fork 33
Add the ability to load from files via a switch and offline #50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add the ability to load from files via a switch and offline #50
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds the ability to load Google Charts resources from local files instead of requiring an online CDN connection. The implementation introduces an offline parameter that can be set to true to switch from online to offline mode.
Key Changes:
- Added helper functions
getGoogleChartsScriptTag()andgetGoogleChartsLoadStatement()in Utils.ahk to generate appropriate script tags based on online/offline mode - Updated multiple visualization files to use the new helper functions instead of hardcoded CDN URLs
- Included Google Charts library files (loader.js, compiled modules, and CSS) in the Resources/Charts directory
Reviewed changes
Copilot reviewed 13 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| Sources/Framework/Utils.ahk | Added helper functions for generating Google Charts script tags with offline/online mode support |
| Sources/Garage/Setup Workbench.ahk | Updated to use new helper functions for chart script generation |
| Sources/Database/Libraries/TelemetryViewer.ahk | Replaced hardcoded CDN URLs with helper function calls |
| Sources/Database/Libraries/PressuresEditor.ahk | Replaced hardcoded CDN URLs with helper function calls |
| Sources/Assistants/Team Center.ahk | Replaced hardcoded CDN URLs with helper function calls |
| Sources/Assistants/Strategy Workbench.ahk | Replaced hardcoded CDN URLs with helper function calls |
| Sources/Assistants/Solo Center.ahk | Replaced hardcoded CDN URLs with helper function calls |
| Sources/Assistants/Libraries/StrategyViewer.ahk | Replaced hardcoded CDN URLs with helper function calls |
| Sources/Assistants/Libraries/RaceReportViewer.ahk | Replaced hardcoded CDN URLs with helper function calls |
| Resources/Charts/offlineSaving/loader.js | Added Google Charts loader library for offline use |
| Resources/Charts/offlineSaving/js/jsapi_compiled_gauge_module.js | Added compiled gauge module for offline use |
| Resources/Charts/offlineSaving/js/jsapi_compiled_corechart_module.js | Added compiled corechart module for offline use |
| Resources/Charts/offlineSaving/css/util/util.css | Added utility CSS for offline charts |
| Resources/Charts/offlineSaving/css/core/tooltip.css | Added tooltip CSS for offline charts |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| if (offline) { | ||
| local resourcesPath := A_WorkingDir . "\Resources\Charts" | ||
| return '<script type="text/javascript" src="' . resourcesPath . '\loader.js"></script>' |
Copilot
AI
Jan 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The offline mode implementation uses a local file path that won't work in browsers. Browsers require file:// protocol URLs or relative paths for local files, not absolute Windows file paths. The path should be converted to a file:// URL or use a relative path.
| return '<script type="text/javascript" src="' . resourcesPath . '\loader.js"></script>' | |
| local resourcesUrl := "file:///" . StrReplace(resourcesPath, "\","/") | |
| return '<script type="text/javascript" src="' . resourcesUrl . '/loader.js"></script>' |
| ;;; Google Charts Helper Functions ;;; | ||
| ;;;-------------------------------------------------------------------------;;; | ||
|
|
||
| getGoogleChartsScriptTag(offline := false) { |
Copilot
AI
Jan 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The offline parameter defaults to false, but there's no documentation explaining how users should set this to true. Consider adding documentation or examples showing how to configure offline mode globally or per-function call.
| height := this.SettingsViewer.getHeight() - 4 | ||
| info := "" |
Copilot
AI
Jan 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The removed height calculation and info retrieval logic appears unrelated to the chart loading changes. These modifications should be in a separate commit focused on layout adjustments rather than mixed with the offline chart loading feature.
| )" | ||
| script := substituteVariables(script, {tableCSS: getTableCSS() | ||
| script := substituteVariables(script, {chartScript: getGoogleChartsScriptTag() |
Copilot
AI
Jan 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent indentation in the substituteVariables call. Lines 5579-5580 use tabs while line 5846 continues with spaces, creating mixed whitespace that reduces code readability.
| script := substituteVariables(script, {chartScript: getGoogleChartsScriptTag() | |
| script := substituteVariables(script, {chartScript: getGoogleChartsScriptTag() |
Added the charts files and to change from using online to offline you have to offline to true