Next release#1697
Conversation
📝 WalkthroughWalkthroughThe PR updates app startup so core and plugin strings are initialized before page data loads, adds a separate plugin-string cache step, adds UI translation tests, renames and reorders scan-device handling, updates one missing-settings log message, refreshes README text, adjusts stacking styles, and changes the sync devices file path. ChangesSettings Log Message Update
App Initialization and Plugin String Caching
Scan Device Handling
Docs and Layout Polish
Sync Devices Path
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ 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 |
…ranslation loading
…into next_release
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
front/pluginsCore.php (1)
295-317: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve cache-busting in the plugin JSON helper.
fetchPluginJson()now serves bothplugins.jsonandtable_plugins_stats.json, but unlike the shared query-json helper it does not appendnocache. That can leave plugin tabs/counts stale after regenerated JSON.Proposed fix
async function fetchPluginJson(filename) { - const response = await fetch(`php/server/query_json.php?file=${filename}`); + const params = new URLSearchParams({ + file: filename, + nocache: Date.now().toString() + }); + const response = await fetch(`php/server/query_json.php?${params.toString()}`); if (!response.ok) throw new Error(`Failed to load ${filename}`); return await response.json(); }Also applies to: 390-390
🤖 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/pluginsCore.php` around lines 295 - 317, The shared plugin JSON helper in fetchPluginJson currently fetches files without cache-busting, which can leave plugin tabs and counts stale after regeneration. Update fetchPluginJson so it appends the nocache flag the same way the existing query-json flow does, and make sure the callers that load plugins.json and table_plugins_stats.json continue to use that helper unchanged. Keep the fix localized around fetchPluginJson and the load path that invokes generateTabs().front/js/cache.js (1)
313-343: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDon’t early-return with a stale string count.
Line 313 skips loading whenever
cacheStrings_v2_completedis true, butisAppInitialized()still requiresSTRINGS_COUNTto match the current language. If the language changes or the count is stale, this path can leave app init permanently inconsistent.Proposed fix
function cacheStrings() { return new Promise((resolve, reject) => { - if(getCache(CACHE_KEYS.initFlag('cacheStrings_v2')) === "true") + const expectedStringCount = getLangCode() == 'en_us' ? 1 : 2; + const cachedStringCount = parseInt(getCache(CACHE_KEYS.STRINGS_COUNT) || '0', 10); + + if(getCache(CACHE_KEYS.initFlag('cacheStrings_v2')) === "true" && cachedStringCount === expectedStringCount) { // Core strings are already cached. Plugin strings are now handled by // the dedicated cachePluginStrings() step — nothing to do here. resolve(); return; } + + setCache(CACHE_KEYS.STRINGS_COUNT, 0); // Create a promise for each language (include en_us by default as fallback) languagesToLoad = ['en_us']🤖 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/js/cache.js` around lines 313 - 343, The early return in cacheStrings based on CACHE_KEYS.initFlag('cacheStrings_v2') can leave STRINGS_COUNT stale for the current language. Update cacheStrings so it still verifies the active language/count state before skipping, or recomputes the core-string count when the language changes, ensuring isAppInitialized() sees the correct STRINGS_COUNT. Use the existing cacheStrings_v2 / handleSuccess flow and keep the cache consistent across language switches.
🧹 Nitpick comments (1)
test/ui/test_ui_translations.py (1)
164-166: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueHardcoded
time.sleep(2)for render wait.Fixed sleeps are a common source of flaky CI runs; consider an explicit
WebDriverWaitcondition on the tab elements' text content instead.♻️ Example using explicit wait instead of sleep
- # Give the tabs a moment to render after init completes - time.sleep(2) + # Wait until tab labels are populated (non-empty) after init completes + WebDriverWait(driver, 5).until( + lambda d: any( + el.text.strip() for el in d.find_elements(By.CSS_SELECTOR, "`#tabs-location` a, .tab-label, h5") + ) + )🤖 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 `@test/ui/test_ui_translations.py` around lines 164 - 166, The UI translation test still uses a fixed time.sleep(2) after init, which can make the test flaky. Replace the hardcoded pause in the test_ui_translations flow with an explicit WebDriverWait that waits on the tab elements’ text/content state to be ready, using the existing tab-locating logic in this test instead of relying on time.sleep.
🤖 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/js/cache.js`:
- Around line 193-201: The `fetchJson('plugins.json')` path in `cache.js` is
collapsing a failed manifest fetch into an empty array, which makes
`pluginsData` look like “no plugins” instead of “manifest unavailable.” Update
the `initFlag('cachePluginStrings_v1')`/`pluginsData` flow so fetch failure is
tracked separately from an empty manifest, and keep the retry logic in the
plugin-strings completion path that currently treats empty `pluginsData` as
done. Make sure the handling in the `plugins.json` loader and the completion
logic around the `pluginsData` checks both distinguish failure from a legitimate
empty plugin list.
In `@test/ui/test_ui_translations.py`:
- Around line 174-176: The assert message strings in the translation test are
plain text and should not use f-string syntax; remove the unnecessary f prefixes
in the relevant assertion(s) in test_ui_translations.py so the messages remain
normal string literals. Use the surrounding assertion text near
getData()/plugins.php to locate the exact statements and keep the message
content unchanged.
---
Outside diff comments:
In `@front/js/cache.js`:
- Around line 313-343: The early return in cacheStrings based on
CACHE_KEYS.initFlag('cacheStrings_v2') can leave STRINGS_COUNT stale for the
current language. Update cacheStrings so it still verifies the active
language/count state before skipping, or recomputes the core-string count when
the language changes, ensuring isAppInitialized() sees the correct
STRINGS_COUNT. Use the existing cacheStrings_v2 / handleSuccess flow and keep
the cache consistent across language switches.
In `@front/pluginsCore.php`:
- Around line 295-317: The shared plugin JSON helper in fetchPluginJson
currently fetches files without cache-busting, which can leave plugin tabs and
counts stale after regeneration. Update fetchPluginJson so it appends the
nocache flag the same way the existing query-json flow does, and make sure the
callers that load plugins.json and table_plugins_stats.json continue to use that
helper unchanged. Keep the fix localized around fetchPluginJson and the load
path that invokes generateTabs().
---
Nitpick comments:
In `@test/ui/test_ui_translations.py`:
- Around line 164-166: The UI translation test still uses a fixed time.sleep(2)
after init, which can make the test flaky. Replace the hardcoded pause in the
test_ui_translations flow with an explicit WebDriverWait that waits on the tab
elements’ text/content state to be ready, using the existing tab-locating logic
in this test instead of relying on time.sleep.
🪄 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: 70a9b5a8-62dd-4d5b-b634-9b27de2206e2
📒 Files selected for processing (6)
front/js/app-init.jsfront/js/cache.jsfront/multiEditCore.phpfront/pluginsCore.phpfront/workflowsCore.phptest/ui/test_ui_translations.py
…n and improve fetch logic; enhance UI tests for translation loading
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
front/js/cache.js (1)
205-208: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRetry the manifest when
pluginsDatais unknown.Line 205 swallows a failed
plugins.jsonrefresh, but Line 410 later treatspluginsData === nullas “plugins may exist” and rejects empty plugin strings without ever re-fetching the manifest. A transient manifest miss can leave no-plugin installs stuck waiting oncachePluginStrings_v1.Proposed fix
.catch(() => { // Fetch failed — leave pluginsData as null (unknown state) so // cachePluginStrings() retries rather than treating it as no-op. - resolve(); + reject(new Error('plugins.json unavailable')); });Or re-fetch
plugins.jsoninsidecachePluginStrings()whenpluginsData === nullbefore deciding whether empty plugin strings are valid.Also applies to: 410-413
🤖 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/js/cache.js` around lines 205 - 208, `cachePluginStrings()` is treating `pluginsData === null` as a terminal “maybe plugins exist” state without retrying the manifest, so a transient `plugins.json` miss can block no-plugin installs. Update the refresh flow in `cache.js` so that when `pluginsData` is null, `cachePluginStrings()` re-fetches the manifest (or otherwise retries the `plugins.json` load) before rejecting empty plugin strings; keep the existing `cachePluginStrings_v1` and `pluginsData` behavior aligned with the retry path.
🤖 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/js/cache.js`:
- Around line 323-328: The STRINGS_COUNT check in cache.js only distinguishes
en_us from non-en_us, so a language change between two non-English locales can
incorrectly satisfy the initialized state. Update the cache logic around the
STRINGS_COUNT / isAppInitialized() path to also track the active language using
a dedicated cache key such as cacheStrings_v2_language, and compare it against
getLangCode() before treating the cache as valid. Set that language marker only
after the language-loading promises in the cache strings flow complete
successfully, so switching locales forces a real reload instead of reusing stale
cached strings.
---
Duplicate comments:
In `@front/js/cache.js`:
- Around line 205-208: `cachePluginStrings()` is treating `pluginsData === null`
as a terminal “maybe plugins exist” state without retrying the manifest, so a
transient `plugins.json` miss can block no-plugin installs. Update the refresh
flow in `cache.js` so that when `pluginsData` is null, `cachePluginStrings()`
re-fetches the manifest (or otherwise retries the `plugins.json` load) before
rejecting empty plugin strings; keep the existing `cachePluginStrings_v1` and
`pluginsData` behavior aligned with the retry path.
🪄 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: 162a599c-eaa8-4868-99e0-46af71dc8082
📒 Files selected for processing (3)
front/js/cache.jsfront/pluginsCore.phptest/ui/test_ui_translations.py
🚧 Files skipped from review as they are similar to previous changes (2)
- test/ui/test_ui_translations.py
- front/pluginsCore.php
… validation; improve stale cache detection logic
There was a problem hiding this comment.
🧹 Nitpick comments (1)
front/css/app.css (1)
248-252: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFooter and sidebar share the same z-index (10000).
Both
.main-footerand.main-sidebar, .left-sideare set toz-index: 10000, one level below.fixed .main-header(10001). If these two ever overlap (e.g., during responsive/collapsed states), their relative stacking will fall back to DOM order rather than explicit intent. Consider giving them distinct values to make the stacking order unambiguous.Also applies to: 335-339
🤖 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 248 - 252, The stacking order for .main-footer and .main-sidebar/.left-side is ambiguous because they share the same z-index value in app.css. Update the z-index values in the affected CSS rules so the footer and sidebar have distinct, intentional layering relative to .fixed .main-header, and keep the changes consistent across both occurrences mentioned in the review.
🤖 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.
Nitpick comments:
In `@front/css/app.css`:
- Around line 248-252: The stacking order for .main-footer and
.main-sidebar/.left-side is ambiguous because they share the same z-index value
in app.css. Update the z-index values in the affected CSS rules so the footer
and sidebar have distinct, intentional layering relative to .fixed .main-header,
and keep the changes consistent across both occurrences mentioned in the review.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0f896453-440f-4db0-9d5b-23f72f24afd6
📒 Files selected for processing (3)
README.mdfront/css/app.cssfront/js/cache.js
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- front/js/cache.js
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
front/plugins/sync/sync.py (1)
109-110: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated
NETALERTX_APIfallback logic.
server/config_paths.pyalready centralizes this exactNETALERTX_API→/tmp/apiresolution asAPI_PATH. Reimplementing it here withos.environ.get(...)risks drift if the shared resolution logic changes (e.g., ifAPI_PATHstarts resolving relative to a configurableNETALERTX_TMPbase instead of a hardcoded/tmp/api).♻️ Suggested refactor
- api_path = os.environ.get('NETALERTX_API', '/tmp/api') - file_path = f"{api_path}/table_devices.json" + file_path = f"{API_PATH}/table_devices.json"(requires importing
API_PATHfromserver.config_pathsat module top level)🤖 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/plugins/sync/sync.py` around lines 109 - 110, The sync path construction in the module-level logic duplicates the NETALERTX_API fallback instead of using the केंदralized resolution already provided by API_PATH. Update the code that builds file_path so it imports and uses API_PATH from server.config_paths, keeping the table_devices.json path derived from that shared value and avoiding a second os.environ.get(...) fallback in this module.front/css/app.css (1)
248-252: 📐 Maintainability & Code Quality | 🔵 TrivialConsider a shared z-index scale.
The new values (10000, 10001, 11051, 12000) are ad-hoc magic numbers scattered across the stylesheet. A small set of CSS custom properties (e.g.
--z-sidebar,--z-header,--z-modal) would make the stacking hierarchy self-documenting and easier to maintain as more layers are added.Also applies to: 321-324, 335-339, 735-739, 1859-1859
🤖 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 248 - 252, The stylesheet is using scattered magic z-index values across the footer and other layers, so replace them with a shared stacking scale defined as CSS custom properties. Add centralized variables for the key layers used by the affected rules, then update the .main-footer and the other referenced selectors to consume those variables instead of hardcoded values so the hierarchy stays consistent and easier to maintain.
🤖 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/css/app.css`:
- Line 1859: The `#modal-ok` rule in app.css is meant to be the topmost modal but
its current z-index is lower than the general .modal rule, so it overrides the
intended stacking order for the modal-ok element. Update the `#modal-ok` selector
so it uses a higher z-index than .modal while preserving the “highest priority”
intent, and fix the comment formatting to satisfy Stylelint by adding the
required whitespace inside the comment delimiters.
- Around line 735-739: The modal stacking in the app stylesheet is too high and
can break Bootstrap overlay ordering. Update the .modal z-index in the CSS so it
stays aligned with Bootstrap’s default overlay layers, and verify that
tooltip/popover behavior still works correctly with the modal class and any
related overlay rules.
In `@front/plugins/sync/sync.py`:
- Around line 120-121: The missing-file branch in sync push is referencing an
undefined variable, causing a NameError before the function can complete. Update
the logging in the else branch of the sync flow to use the existing file path
variable instead of file_content, matching the earlier error handling pattern
used in the same function. Locate the fix in the logic around the mylog call
inside the missing-file path handling for the sync plugin.
---
Nitpick comments:
In `@front/css/app.css`:
- Around line 248-252: The stylesheet is using scattered magic z-index values
across the footer and other layers, so replace them with a shared stacking scale
defined as CSS custom properties. Add centralized variables for the key layers
used by the affected rules, then update the .main-footer and the other
referenced selectors to consume those variables instead of hardcoded values so
the hierarchy stays consistent and easier to maintain.
In `@front/plugins/sync/sync.py`:
- Around line 109-110: The sync path construction in the module-level logic
duplicates the NETALERTX_API fallback instead of using the केंदralized
resolution already provided by API_PATH. Update the code that builds file_path
so it imports and uses API_PATH from server.config_paths, keeping the
table_devices.json path derived from that shared value and avoiding a second
os.environ.get(...) fallback in this module.
🪄 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: 7775fe10-1577-410a-a0e0-6d9c4b63a9cc
📒 Files selected for processing (2)
front/css/app.cssfront/plugins/sync/sync.py
| .modal | ||
| { | ||
| z-index: 12000; | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -B1 -A3 '\.modal-backdrop' front/css/app.cssRepository: netalertx/NetAlertX
Length of output: 157
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Files containing modal-backdrop ==\n'
rg -n --hidden --glob '!**/node_modules/**' '\.modal-backdrop' .
printf '\n== Files containing modal / tooltip / toast z-index rules ==\n'
rg -n --hidden --glob '!**/node_modules/**' 'z-index:\s*[0-9]+' front/css . | sed -n '1,200p'
printf '\n== Relevant CSS files around front/css/app.css area ==\n'
wc -l front/css/app.css
sed -n '700,780p' front/css/app.cssRepository: netalertx/NetAlertX
Length of output: 50377
🏁 Script executed:
python3 - <<'PY'
import re
from pathlib import Path
files = [
Path("front/css/app.css"),
Path("front/lib/bootstrap/bootstrap.min.css"),
Path("front/lib/datatables/datatables.css"),
]
selectors = [".modal", ".modal-backdrop", ".tooltip", ".popover", ".dropdown-menu", ".toast"]
for path in files:
text = path.read_text(errors="ignore")
print(f"\n== {path} ==")
for sel in selectors:
idx = text.find(sel)
if idx == -1:
continue
snippet = text[idx:idx+300]
# compress whitespace for readability
compact = re.sub(r"\s+", " ", snippet)
m = re.search(r"z-index\s*:\s*([^;}]*)", compact)
print(f"{sel}: {m.group(1) if m else 'no z-index in nearby snippet'}")
print(compact[:220])
PYRepository: netalertx/NetAlertX
Length of output: 3413
Keep modal stacking aligned with Bootstrap overlays. z-index: 12000 will push modals above the default tooltip/popover layers (1060/1070), so those elements may render under the dialog unless they’re adjusted too.
🤖 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 735 - 739, The modal stacking in the app
stylesheet is too high and can break Bootstrap overlay ordering. Update the
.modal z-index in the CSS so it stays aligned with Bootstrap’s default overlay
layers, and verify that tooltip/popover behavior still works correctly with the
modal class and any related overlay rules.
| #modal-ok | ||
| { | ||
| z-index: 1051; /*highest priority*/ | ||
| z-index: 11051; /*highest priority*/ |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
#modal-ok z-index is lower than the general .modal rule, contradicting its "highest priority" comment.
#modal-ok carries class="modal" (per front/php/templates/modals.php), and since ID selectors outrank class selectors in specificity, this rule's z-index: 11051 overrides the newly-added .modal { z-index: 12000; } for that element — making the "OK" modal render below any other element matched only by .modal. That's the opposite of the stated intent.
Also flagged by Stylelint: missing whitespace inside the comment delimiters.
🐛 Proposed fix
`#modal-ok`
{
- z-index: 11051; /*highest priority*/
+ z-index: 12000; /* highest priority */
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| z-index: 11051; /*highest priority*/ | |
| z-index: 12000; /* highest priority */ |
🧰 Tools
🪛 Stylelint (17.14.0)
[error] 1859-1859: Expected whitespace after "/*" (comment-whitespace-inside)
(comment-whitespace-inside)
[error] 1859-1859: Expected whitespace before "*/" (comment-whitespace-inside)
(comment-whitespace-inside)
🤖 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` at line 1859, The `#modal-ok` rule in app.css is meant to be
the topmost modal but its current z-index is lower than the general .modal rule,
so it overrides the intended stacking order for the modal-ok element. Update the
`#modal-ok` selector so it uses a higher z-index than .modal while preserving the
“highest priority” intent, and fix the comment formatting to satisfy Stylelint
by adding the required whitespace inside the comment delimiters.
Source: Linters/SAST tools
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
front/css/app.css (2)
321-324: 📐 Maintainability & Code Quality | 🔵 Trivial
.fixed .main-headernow outranks the sidebar/footer by only 1.
z-index: 10001is just one unit above the sidebar/footer's10000, leaving very little headroom before this ordering silently breaks with future additions (dropdowns, mini-sidebar hover panel, etc.). AdminLTE's own defaults keep header/sidebar much lower (around 1030/810) specifically to avoid this kind of collision with Bootstrap overlay components.🤖 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 321 - 324, The .fixed .main-header rule is using a z-index that is too close to the sidebar/footer stack, which leaves little room for future overlay layers. Update the z-index in the .fixed .main-header selector in app.css to a safer value consistent with the existing AdminLTE layering strategy, and keep it clearly above the sidebar/footer without narrowly incrementing by 1 so other UI layers can be added without collisions.
248-252: 📐 Maintainability & Code Quality | 🔵 TrivialDuplicate of prior stacking concern; also part of broader z-index inflation.
This footer rule sets
z-index: 10000, following the same pattern flagged previously for.modal/#modal-ok. Combined with the header (10001) and sidebar (10000) changes below, the stylesheet now uses arbitrarily large, closely-spaced z-index values across layout regions and overlays, which increases the risk of one region unintentionally sitting above dropdowns/tooltips/modals that rely on Bootstrap's much lower default scale (~1000-1070).Consider introducing a small set of CSS custom properties (e.g.
--z-header,--z-sidebar,--z-footer,--z-modal) to keep the stacking order self-documenting and easy to audit going forward.🤖 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 248 - 252, The `.main-footer` stacking rule is part of the broader z-index inflation issue; replace the hardcoded `z-index: 10000` with a named CSS custom property used consistently across the layout. Update the related stacking declarations in the stylesheet (including the header/sidebar/modal rules) to reference a small, self-documenting z-index scale such as `--z-header`, `--z-sidebar`, `--z-footer`, and `--z-modal`, so the order stays clear and easier to maintain.
🤖 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 `@server/scan/session_events.py`:
- Around line 38-46: Move the own-device insertion in process_scan so
save_own_device(db) runs after exclude_ignored_devices(db), not before. The
issue is in the scan pipeline ordering inside the session_events processing
flow: broad ignored-device rules can remove the local host if it is inserted too
early. Keep the exclusion step first, then call save_own_device(db) so the own
device remains present in CurrentScan for the rest of the logic.
---
Nitpick comments:
In `@front/css/app.css`:
- Around line 321-324: The .fixed .main-header rule is using a z-index that is
too close to the sidebar/footer stack, which leaves little room for future
overlay layers. Update the z-index in the .fixed .main-header selector in
app.css to a safer value consistent with the existing AdminLTE layering
strategy, and keep it clearly above the sidebar/footer without narrowly
incrementing by 1 so other UI layers can be added without collisions.
- Around line 248-252: The `.main-footer` stacking rule is part of the broader
z-index inflation issue; replace the hardcoded `z-index: 10000` with a named CSS
custom property used consistently across the layout. Update the related stacking
declarations in the stylesheet (including the header/sidebar/modal rules) to
reference a small, self-documenting z-index scale such as `--z-header`,
`--z-sidebar`, `--z-footer`, and `--z-modal`, so the order stays clear and
easier to maintain.
🪄 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: 6b28dd56-9844-4f2c-a518-1970f0b32554
📒 Files selected for processing (4)
front/css/app.cssfront/plugins/sync/sync.pyserver/scan/device_handling.pyserver/scan/session_events.py
1 participant
Summary by CodeRabbit