Next release#1679
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughAdds shimmer skeleton loading UIs across all major NetAlertX pages (Settings, Devices, Events, Device Details tabs, Plugins, Workflows, Presence, Report, Notifications, Maintenance, SystemInfo). Introduces 20+ PHP skeleton templates, a shared CSS system with animation and dark-theme overrides, per-page JS hide helpers with 15-second fallbacks, spinner z-index fixes, and consolidates ChangesSkeleton Loading UI – Full Site
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
front/settings.php (1)
328-344:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPotential runtime error if
sectionHtmlis undefined.Line 330 calls
sectionHtml.trim()without checking ifsectionHtmlis defined. If theoverviewSectionsandoverviewSectionsHtmlarrays have mismatched lengths or ifindexgoes out of bounds,overviewSectionsHtml[index]will beundefined, causing a TypeError when.trim()is called.🛡️ Add defensive check for undefined
const sectionHtml = overviewSectionsHtml[index]; - if (sectionHtml.trim()) { + if (sectionHtml && sectionHtml.trim()) { overviewSections_html += `<div class="overview-section col-sm-12" id="${section}">🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@front/settings.php` around lines 328 - 344, The code at line 330 calls sectionHtml.trim() without verifying that sectionHtml is defined, which will cause a TypeError if overviewSectionsHtml[index] returns undefined due to array length mismatch or index out of bounds. Before calling .trim() on sectionHtml after retrieving it from overviewSectionsHtml[index], add a null/undefined check to ensure the variable exists and is not undefined. You can combine this check with the existing trim() validation by verifying sectionHtml is defined before attempting to call any methods on it.
🧹 Nitpick comments (1)
front/css/app.css (1)
2605-2608: ⚡ Quick winKeyframe name should use kebab-case convention.
The keyframe name
settingsShimmeruses camelCase, which deviates from the CSS naming convention of kebab-case. Usingsettings-shimmerwould align with standard CSS practices.♻️ Rename to kebab-case
-@keyframes settingsShimmer { +@keyframes settings-shimmer { 0% { background-position: -600px 0; } 100% { background-position: 600px 0; } }Also update the animation reference on line 2635:
- animation: settingsShimmer 1.5s infinite linear; + animation: settings-shimmer 1.5s infinite linear;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@front/css/app.css` around lines 2605 - 2608, The keyframe animation name `settingsShimmer` uses camelCase instead of the CSS convention of kebab-case. Rename the `@keyframes settingsShimmer` definition to `@keyframes settings-shimmer` and update any animation references that use this keyframe (such as the animation-name property on line 2635) to also use the kebab-case name `settings-shimmer`.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@front/php/templates/settings_skeleton.php`:
- Around line 11-13: Update the column classes in all skel-overview-card divs to
match the dynamically generated plugin cards in settings_utils.js. Replace the
current classes col-xs-6 col-sm-4 col-md-3 col-lg-2 col-xxl-1 with col-xs-12
col-sm-6 col-md-4 col-lg-3 for each skel-overview-card element. This change
should be applied to both the skeleton cards in lines 11-13 and the additional
cards in lines 20-22 to prevent layout shift when real content replaces the
skeleton.
---
Outside diff comments:
In `@front/settings.php`:
- Around line 328-344: The code at line 330 calls sectionHtml.trim() without
verifying that sectionHtml is defined, which will cause a TypeError if
overviewSectionsHtml[index] returns undefined due to array length mismatch or
index out of bounds. Before calling .trim() on sectionHtml after retrieving it
from overviewSectionsHtml[index], add a null/undefined check to ensure the
variable exists and is not undefined. You can combine this check with the
existing trim() validation by verifying sectionHtml is defined before attempting
to call any methods on it.
---
Nitpick comments:
In `@front/css/app.css`:
- Around line 2605-2608: The keyframe animation name `settingsShimmer` uses
camelCase instead of the CSS convention of kebab-case. Rename the `@keyframes
settingsShimmer` definition to `@keyframes settings-shimmer` and update any
animation references that use this keyframe (such as the animation-name property
on line 2635) to also use the kebab-case name `settings-shimmer`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3cd3659e-b516-4e9f-83f8-6b11b97dcd90
📒 Files selected for processing (8)
front/css/app.cssfront/css/dark-patch.cssfront/css/system-dark-patch.cssfront/js/common.jsfront/js/settings_utils.jsfront/php/templates/header.phpfront/php/templates/settings_skeleton.phpfront/settings.php
- Created skeleton templates for events, notifications, plugins, presence, report, settings, and workflows to enhance loading states. - Implemented structured skeletons for tab details, tab events, maintenance backup, database tools, logging, multi-edit, presence, sessions, and system info. - Each template includes shimmer effects for loading indicators, improving user experience during data fetching.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
front/php/templates/skel_devices.php (1)
5-71: ⚡ Quick winRefactor repeated skeleton blocks into loops to prevent drift.
This template has heavy copy/paste repetition (tiles + many table rows). Converting these blocks to loop-generated markup will keep future tweaks consistent and reduce accidental mismatches.
♻️ Example refactor pattern
- <div class="row"> - <div class="col-lg-2 col-sm-4 col-xs-6 padding-5px"> - <div class="skel-tile"> - ... - </div> - </div> - ... (repeated 6x) - </div> + <div class="row"> + <?php for ($i = 0; $i < 6; $i++): ?> + <div class="col-lg-2 col-sm-4 col-xs-6 padding-5px"> + <div class="skel-tile"> + <div class="skel-tile-inner"> + <div class="skel-tile-num skel-shimmer"></div> + <div class="skel-tile-label skel-shimmer"></div> + </div> + <div class="skel-tile-icon-area"> + <div class="skel-tile-icon-shape skel-shimmer"></div> + </div> + </div> + </div> + <?php endfor; ?> + </div> ... - <div class="skel-tr">...</div> - ... (repeated many times) + <?php for ($r = 0; $r < 20; $r++): ?> + <div class="skel-tr"> + <span class="skel-td skel-shimmer" style="flex:0.5"></span> + <?php for ($c = 0; $c < 5; $c++): ?> + <span class="skel-td skel-shimmer"></span> + <?php endfor; ?> + </div> + <?php endfor; ?>Also applies to: 111-294
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@front/php/templates/skel_devices.php` around lines 5 - 71, The skel_devices.php template contains six identical skeleton tile blocks with col-lg-2 col-sm-4 col-xs-6 padding-5px divs that are repeated verbatim within the row. Replace these six hardcoded blocks with a PHP loop (such as a for loop from 1 to 6) that generates each identical skel-tile block dynamically. This refactoring should also be applied to the other repetitive sections mentioned in lines 111-294 (such as table rows with the same pattern) to maintain consistency across the entire template and prevent future maintenance issues caused by copy-paste drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@front/events.php`:
- Around line 264-266: The anonymous function being assigned to window.onload in
the load handler registration overwrites any existing onload handlers, which can
break other initialization code. Replace the `window.onload = function () {
setTimeout(hideEventsSkeleton, 15000); }` assignment with
`window.addEventListener('load', function () { setTimeout(hideEventsSkeleton,
15000); })` to register the event handler non-destructively without overwriting
other handlers that may have already been registered.
In `@front/php/templates/footer.php`:
- Around line 46-48: The require statement for the modals template is using an
incorrect relative path that includes unnecessary directory nesting. Since
footer.php and modals.php are both located in the same templates directory,
change the require path from 'php/templates/modals.php' to simply 'modals.php'
to correctly reference the sibling file. This matches the correct pattern used
elsewhere in the file where other sibling files like migrationCheck.php are
required without the duplicate directory path.
In `@front/pluginsCore.php`:
- Around line 863-869: The current timeout handler only calls
hidePluginsSkeleton() but does not hide the spinner, which can leave the overlay
stuck if getData() fails. Modify the window load event listener's setTimeout
callback to call both hidePluginsSkeleton() and hideSpinner() to ensure the
spinner is removed in the fallback scenario. Additionally, locate the catch
block in the getData() function and add a hideSpinner() call there to
immediately remove the spinner on error, preventing the overlay from blocking
the page when the data fetch fails.
In `@front/presence.php`:
- Around line 512-518: The hidePresenceSkeleton function currently only removes
the `#presence-skeleton` element via jQuery fadeOut and remove, but does not clean
up the spinner element. To prevent UI lock when presence loading fails to
complete, modify the hidePresenceSkeleton function to also call hideSpinner()
within the completion callback of the fadeOut animation, ensuring both the
skeleton and spinner are cleared when the 15-second timeout fallback triggers.
In `@front/userNotifications.php`:
- Around line 224-226: The code directly assigns to window.onload which
overwrites any previously registered onload handlers. Instead of using direct
assignment in the onload handler that calls setTimeout with
hideNotificationsSkeleton, use window.addEventListener with the 'load' event to
register the callback additively. This ensures that other onload initializers
remain intact and your notification skeleton hiding logic executes without
overriding existing functionality.
In `@front/workflowsCore.php`:
- Around line 1380-1386: The hideWorkflowsSkeleton function currently only
removes the skeleton element but does not clear the spinner overlay. When the
15-second fallback timeout executes (because the workflows request stalled and
`.always()` never runs), both the skeleton and the spinner need to be hidden.
Add a call to hideSpinner() within the hideWorkflowsSkeleton function to ensure
the spinner overlay is cleared along with the skeleton when the timeout
triggers.
---
Nitpick comments:
In `@front/php/templates/skel_devices.php`:
- Around line 5-71: The skel_devices.php template contains six identical
skeleton tile blocks with col-lg-2 col-sm-4 col-xs-6 padding-5px divs that are
repeated verbatim within the row. Replace these six hardcoded blocks with a PHP
loop (such as a for loop from 1 to 6) that generates each identical skel-tile
block dynamically. This refactoring should also be applied to the other
repetitive sections mentioned in lines 111-294 (such as table rows with the same
pattern) to maintain consistency across the entire template and prevent future
maintenance issues caused by copy-paste drift.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1b5272a8-8815-4e1c-a78b-c44537df7c9b
📒 Files selected for processing (43)
front/appEvents.phpfront/css/app.cssfront/deviceDetails.phpfront/deviceDetailsEdit.phpfront/deviceDetailsEvents.phpfront/deviceDetailsPresence.phpfront/deviceDetailsSessions.phpfront/devices.phpfront/events.phpfront/maintenance.phpfront/network.phpfront/php/templates/footer.phpfront/php/templates/migrationCheck.phpfront/php/templates/skel_device_details.phpfront/php/templates/skel_devices.phpfront/php/templates/skel_events.phpfront/php/templates/skel_notifications.phpfront/php/templates/skel_plugins.phpfront/php/templates/skel_presence.phpfront/php/templates/skel_report.phpfront/php/templates/skel_settings.phpfront/php/templates/skel_tab_details.phpfront/php/templates/skel_tab_events.phpfront/php/templates/skel_tab_maint_backup.phpfront/php/templates/skel_tab_maint_dbtools.phpfront/php/templates/skel_tab_maint_logging.phpfront/php/templates/skel_tab_maint_multiedit.phpfront/php/templates/skel_tab_presence.phpfront/php/templates/skel_tab_sessions.phpfront/php/templates/skel_tab_sysinfo_initcheck.phpfront/php/templates/skel_tab_sysinfo_network.phpfront/php/templates/skel_tab_sysinfo_server.phpfront/php/templates/skel_tab_sysinfo_storage.phpfront/php/templates/skel_workflows.phpfront/plugins.phpfront/pluginsCore.phpfront/presence.phpfront/report.phpfront/settings.phpfront/systeminfo.phpfront/userNotifications.phpfront/workflows.phpfront/workflowsCore.php
💤 Files with no reviewable changes (2)
- front/php/templates/skel_settings.php
- front/network.php
✅ Files skipped from review due to trivial changes (11)
- front/php/templates/skel_tab_maint_multiedit.php
- front/php/templates/skel_tab_maint_dbtools.php
- front/php/templates/skel_tab_maint_logging.php
- front/php/templates/skel_device_details.php
- front/php/templates/skel_workflows.php
- front/php/templates/skel_tab_maint_backup.php
- front/php/templates/skel_tab_details.php
- front/php/templates/skel_tab_sessions.php
- front/php/templates/migrationCheck.php
- front/php/templates/skel_events.php
- front/php/templates/skel_tab_events.php
🚧 Files skipped from review as they are similar to previous changes (2)
- front/settings.php
- front/css/app.css
…dling and hide spinner in skeleton functions
This was referenced Jun 21, 2026
Merged
Merged
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary by CodeRabbit