Conversation
Test results 20 files 20 suites 24m 38s ⏱️ Results for commit 52c74a8. ♻️ This comment has been updated with latest results. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3730 +/- ##
==========================================
+ Coverage 63.23% 63.27% +0.03%
==========================================
Files 617 618 +1
Lines 45662 45727 +65
Branches 43 43
==========================================
+ Hits 28874 28933 +59
- Misses 16778 16784 +6
Partials 10 10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
A few annoying bugs:
This dependency is necessary for |
a90fa68 to
c2191ba
Compare
|
This PR can be reviewed separately, but should be merged after all dependencies are approved, as I want to remove the |
|
Manual test:
|
hmpf
left a comment
There was a problem hiding this comment.
@lunkwill42, could you check saving in portadmin?
44ed5d8 to
b9ceec4
Compare
lunkwill42
left a comment
There was a problem hiding this comment.
@lunkwill42, could you check saving in portadmin?
When I get time to set up my environment for it properly, which very likely won't happen until Wednesday this week.
Also, has anyway looked at the SonarQube errors?
f14fb08 to
1e94624
Compare
|
This branch has merged in changes from four different dependency upgrade/replacement PRs. These changes have already been tested and approved. To make it easier to review/pass this mega/meta PR I've created this commit mapping. 
|
|
Some of the SonarCloud complaints here seem to be about the vendored-in plugin code. We should maybe find a way to exclude those from analysis in |
|
I pushed a commit to ensure SonarQube ignores vendored-in code, but it still flags some issues with the code that should be reviewed, fixed or dismissed. Also, not sure how the split-brain JS works, but |
Tested - it appears to work just fine 👍 |
|
Something is slightly off with job duration sparklines in ipdevinfo, though: Average duration is now displayed below the sparkline, instead of to the right, causing increased vertical space usage per row. Original: 
New version: 
|
|
@lunkwill42 I have pushed three commits which address your feedback (and one regression with spot dots on the sparklines):
|
Patch the single jQuery 4 incompatibility ($.camelCase removal) with a native String.replace equivalent. No other library depends on jquery-migrate, so it can be fully removed.
Select2 4.1.0-rc.0 replaced aria-attribute selectors with BEM-style class selectors for option states (selected, highlighted, disabled).
We will never fix linting issues in vendored-in code, so we should make sure SonarCloud ignores it.
61c45cd to
52c74a8
Compare
|
|
@lunkwill42 I removed jQuery as a dependency in the |
lunkwill42
left a comment
There was a problem hiding this comment.
Great! Let's fly with this!
Scope and purpose
Upgrade jQuery from 3.7.1 to 4.0.0.
Changes
$.parseJSON()->JSON.parse()$.trim()->String.prototype.trim()How to test
Open browser DevTools console and verify no JavaScript errors on:
/portadmin/) - test saving port changes/search/room/<room>/) - verify sparklines render/ipdevinfo/<device>/) - verify sparklines render/machinetracker/) - test table sorting, test with 10.0.0.0-10.0.1.255 as range, and 10000 days/seeddb/) - test DataTables filtering/pagination (ip device tab)jQuery Migrate logs warnings to console for deprecated API usage - these are expected for the legacy plugins.
Contributor Checklist