Improve Lucide loading and error recovery #145
Merged
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.
Closes #143.
I confirmed that Lucide recommends using unkpg as a CDN. Personally I'm not a fan of unpkg for production use as they don't guarantee uptime (hence #143) but since this is just for icons and the site doesn't receive a ton of traffic, it's probably fine for now. A simple alternative would be to host lucide.min.js ourselves like we do with htmx, though that has its own drawbacks to consider. The other changes introduced in this PR are still relevant regardless of where we source Lucide from.
I consolidated our Lucide script fetching and initialization tags into the base template, so we don't have to worry about adding them to every page that wants to use icons, and it's easier to maintain the dependency going forward. EDIT: This included updating the article feedback form to use lucide markup in the template instead of using JS to dynamically change the "Yes/No" responses to icons. I added the
asyncattribute to that script tag so it will load in parallel without blocking anything, as #143 was blocking page rendering for several seconds each page load. A risk here is that the page will display without icons if Lucide is taking a long time to load, but IMO that's better than the whole page taking a long time to load because icons are missing, and search engine scoring algorithms agree. I could've useddeferinstead ofasync, butdeferstill blocks theDOMContentLoadedevent until the script finishes, so we would've risked partially blocking interactivity for some pages:portmap/static/js/article.js
Line 10 in fb26c46
Going forward, if Lucide is unable to load for whatever reason, the page will just remain rendered without icons, which is what ultimately happens anyway if Lucide can't load.