Adding sequential resource loading to edit.htm#5306
Conversation
WalkthroughReplaces static inlined asset references in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@wled00/data/edit.htm`:
- Around line 111-123: The loadFiles function can call cb twice because onerror
remains set when the timeout fires; update loadFiles to clear s.onerror (in
addition to s.onload) when the timeout handler runs and also clear both handlers
inside onerror/onload before invoking cb or scheduling retries so no events fire
after the callback; specifically modify the timeout branch that currently sets
s.onload=null to also set s.onerror=null and ensure s.onload and s.onerror are
nulled at the start of their handlers (references: function loadFiles, local
variable s, handlers s.onload and s.onerror, and tm).
🧹 Nitpick comments (1)
wled00/data/edit.htm (1)
125-136: Update Ace editor to a current version.Version 1.23.4 is secure and accessible on cdnjs, but it is significantly outdated (current version is 1.43.5). Consider updating to a recent stable release to benefit from improvements and maintain dependency currency. After updating the version in the code, run
npm run buildto regenerate embedded headers.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@wled00/data/edit.htm`:
- Around line 125-136: The loadFiles callback assumes common.js defines
window.loadResources and thus never calls S() when common.js fails; update the
loadFiles completion handler (the anonymous callback passed when loading
'common.js') to detect when window.loadResources is missing and provide a
fallback: either call S() directly to initialize basic editor functionality or
call a new minimal init/error UI routine that displays a user-facing error and
sets up a degraded editor; adjust the nested CDN-loading block (the callbacks
that load ace.min.js, mode-json.min.js, theme-monokai.min.js) to also call that
same fallback when window.loadResources is undefined so the editor still
initializes or shows an error instead of silently doing nothing.
following up on my other improvements to the UI: edit.htm was still lacking sequential loading of resources. This fixes that and makes the new editor much more ESP8266 friendly. Thanks to @willmmiles for pointing out the deficiency.
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.