-
-
Notifications
You must be signed in to change notification settings - Fork 614
Drop moment.js for luxon.js #3655
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
base: development
Are you sure you want to change the base?
Conversation
Signed-off-by: yubiuser <[email protected]>
Signed-off-by: yubiuser <[email protected]>
Signed-off-by: yubiuser <[email protected]>
Signed-off-by: yubiuser <[email protected]>
1a9b991 to
99d6917
Compare
Signed-off-by: yubiuser <[email protected]>
Signed-off-by: yubiuser <[email protected]>
Signed-off-by: yubiuser <[email protected]>
Signed-off-by: yubiuser <[email protected]>
Signed-off-by: yubiuser <[email protected]>
Signed-off-by: yubiuser <[email protected]>
Signed-off-by: yubiuser <[email protected]>
|
Ready for review and testing |
|
I did not implement any of the new features https://github.com/Wernfried/daterangepicker would offer. This could be implemented in a follow-up PR. |
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 pull request replaces the Moment.js library with Luxon for date/time handling in the Pi-hole Web Interface. The change includes updating Chart.js adapters and removing source map files for vendor libraries.
Key Changes
- Library Migration: Replaces Moment.js with Luxon for date/time operations
- Chart.js Adapter Update: Switches from chartjs-adapter-moment to chartjs-adapter-luxon
- Source Map Cleanup: Removes source map files for moment.js, daterangepicker, and chartjs-adapter-moment
Reviewed Changes
Copilot reviewed 7 out of 18 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| vendor/moment/* | Removes all Moment.js library files (JS and map files) |
| vendor/chartjs-adapter-moment/* | Removes Moment.js-based Chart.js adapter files |
| vendor/chartjs-adapter-luxon/chartjs-adapter-luxon.umd.min.js | Adds Luxon-based Chart.js adapter (new file) |
| vendor/daterangepicker/*.map | Removes source map files for daterangepicker |
| vendor/daterangepicker/daterangepicker.min.css | Adds jsDelivr CDN attribution header |
| scripts/lua/header.lp | Adds luxon.min.js script include |
| scripts/lua/header_authenticated.lp | Replaces moment.js and chartjs-adapter-moment with chartjs-adapter-luxon |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
While using this PR as a base for implementing the new constrained "All Time" range, I found a bug in the daterangepicker fork, and have submitted a PR upstream to get it fixed |
Signed-off-by: yubiuser <[email protected]>
Signed-off-by: yubiuser <[email protected]>
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
What does this PR aim to accomplish?:
This PR initially only tried to replace https://github.com/dangrossman/daterangepicker which has been unmaintained for >5 years now. I found a active fork at https://github.com/Wernfried/daterangepicker which had the nice side effect that it replaced
moment.js(also in maintenance mode) mode with its successorluxon(https://moment.github.io/luxon). I swapped all the libraries and adjusted the code. Not all date/time format could be translated 1:1 but it's close.It was quite straight forward, but one
moment.jsfunction (toHuman()) did not work inluxon.js(with millisecond input it would not translate to human readable output but keep in miliseconds). There is a long-standing luxon bug (moment/luxon#1134) which provided a nice workaround with a wrapper function. I added it, but it required to loadluxon.jsalready inheader.lp(opposed toheader_authenticated).By submitting this pull request, I confirm the following:
git rebase)