#3187 Refactoring of system settings looks #3191
Conversation
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThe PR restructures the footer styling to use fixed positioning with flex layout in both CSS themes, and adds sidebar-related translation keys across ten language files to support new UI components. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @webapp-resources/messages_pl.properties:
- Around line 3598-3603: The new sidebar localization keys
(systemSettings.sidebar.ariaLabel, systemSettings.sidebar.searchPlaceholder,
systemSettings.sidebar.searchAria, systemSettings.sidebar.selectAll,
systemSettings.sidebar.clearAll, systemSettings.sidebar.reset) are left in
English in messages_pl.properties; replace their values with Polish translations
for consistency (suggestions: ariaLabel -> "Nawigacja ustawień systemowych",
searchPlaceholder -> "Szukaj ustawień", searchAria -> "Wyszukaj ustawienia",
selectAll -> "Zaznacz wszystko", clearAll -> "Wyczyść wszystko", reset ->
"Resetuj") so the Polish locale no longer falls back to English.
In @WebContent/assets/common_DARK.css:
- Around line 254-269: #sltsFooter uses a hard-coded left: 260px and light color
values that break the DARK theme; replace the fixed values with theme variables
(use --slts-color-background for background and --slts-color-secondary-lighter
for the border) and mirror the DEFAULT layout by defaulting left to 0 (or to a
sidebar width CSS variable if one exists, e.g., --slts-sidebar-width) so it’s
consistent across themes; add a responsive override (media query) to set left: 0
on narrow viewports or when the sidebar is hidden/collapsed so the footer
doesn’t stay offset on small screens.
In @WebContent/assets/common_DEFAULT.css:
- Around line 262-277: The fixed footer (#sltsFooter) can overlap page content
and pointer-events: none makes it non-interactive and causes click-through
surprises; update styles to reserve space and keep it interactive by removing
pointer-events: none and ensuring the layout accounts for the footer height
(e.g., add body or main content padding-bottom equal to the footer height, or
switch #sltsFooter to position: sticky at bottom) so the footer no longer covers
actionable elements and remains clickable.
🧹 Nitpick comments (1)
webapp-resources/messages_ru.properties (1)
3609-3614: NewsystemSettings.sidebar.*keys look structurally fine; consider RU translation vs. leaving English intentionally.Given RU is likely not actively maintained, English values are probably OK for now, but if RU is user-facing, these should eventually be translated (esp. ARIA strings).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
WebContent/WEB-INF/jsp/systemSettings.jspWebContent/assets/common_DARK.cssWebContent/assets/common_DEFAULT.csswebapp-resources/messages_de.propertieswebapp-resources/messages_en.propertieswebapp-resources/messages_es.propertieswebapp-resources/messages_fi.propertieswebapp-resources/messages_fr.propertieswebapp-resources/messages_lu.propertieswebapp-resources/messages_nl.propertieswebapp-resources/messages_pl.propertieswebapp-resources/messages_pt.propertieswebapp-resources/messages_ru.propertieswebapp-resources/messages_zh.properties
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-05T14:39:51.606Z
Learnt from: Limraj
Repo: SCADA-LTS/Scada-LTS PR: 3239
File: webapp-resources/messages_fi.properties:3565-3565
Timestamp: 2026-01-05T14:39:51.606Z
Learning: SCADA-LTS maintains translations only for Polish and English. Do not invest in or expect active maintenance for other languages (e.g., messages_fi.properties, messages_de.properties). If a messages_*.properties file contains English text and is not maintained, it can be left as-is for now. AI translation may be used in the future. During reviews, prioritize Polish and English keys/text; non-maintained locales should not block changes.
Applied to files:
webapp-resources/messages_pt.propertieswebapp-resources/messages_pl.propertieswebapp-resources/messages_en.propertieswebapp-resources/messages_es.propertieswebapp-resources/messages_fi.propertieswebapp-resources/messages_lu.propertieswebapp-resources/messages_fr.propertieswebapp-resources/messages_nl.propertieswebapp-resources/messages_zh.propertieswebapp-resources/messages_de.propertieswebapp-resources/messages_ru.properties
🔇 Additional comments (11)
webapp-resources/messages_en.properties (2)
3460-3460: Unexpected key outside PR scope?Line 3460 introduces
validate.unsupportedDataType, which appears unrelated to the sidebar refactoring described in the PR objectives. Please clarify if this belongs in this commit or should be separated into a distinct change for data validation improvements.
3461-3466: ✓ New sidebar translation keys are clear and well-structured.The six new keys under
systemSettings.sidebar.*align well with the PR objectives for the new sidebar UI component:
- Aria labels (
ariaLabel,searchAria) support accessibility for screen readers- Placeholder and button labels are concise and user-friendly
- Consistent lowercase style matches existing UI conventions
- Keys follow the hierarchical naming pattern (
systemSettings.sidebar.*)Per the learnings, translations in maintained locales (Polish & English) are the priority; other locales can be addressed separately or via future AI-assisted translation.
webapp-resources/messages_pt.properties (1)
3613-3618: English text in Portuguese localization file.The newly added sidebar translation keys contain English values rather than Portuguese translations. While this file is not actively maintained per project standards, the values should ideally be in Portuguese or left as placeholders for future AI translation.
Given the learnings that Portuguese is not a maintained locale, this is acceptable for now, but be aware that these keys use English text.
⛔ Skipped due to learnings
Learnt from: Limraj Repo: SCADA-LTS/Scada-LTS PR: 3239 File: webapp-resources/messages_fi.properties:3565-3565 Timestamp: 2026-01-05T14:39:59.665Z Learning: The Scada-LTS project's translation policy focuses only on Polish and English translations. Other language files (such as messages_fi.properties, messages_de.properties, etc.) may contain English text and are not actively maintained per project guidelines. AI translation may be used for these files in the future.webapp-resources/messages_es.properties (1)
3500-3506: New sidebar localization keys added; note non-maintained locale status.The changes introduce seven new localization keys for the sidebar feature (lines 3501–3506) and re-add
validate.unsupportedDataType(line 3500). All keys follow the correct format and naming convention consistent with the file structure.However, per the retrieved learnings, Spanish is a non-maintained locale in SCADA-LTS—only Polish and English are actively maintained. The new keys currently contain English text values rather than Spanish translations. This is acceptable for now per the maintenance policy, as AI translation may be applied in the future during maintenance cycles.
Recommendation: Document or track these keys (and similar English-only entries in non-maintained locales) for future translation work to improve the Spanish localization.
webapp-resources/messages_fr.properties (1)
3454-3460: Note: Sidebar translation keys contain English text instead of French.Lines 3454–3460 add new sidebar-related translation keys, but the values are in English rather than French. While this is inconsistent with the file's primary purpose as a French translation file, this is acceptable given that SCADA-LTS maintains translations only for Polish and English, and non-maintained locales like French can be left with English text as placeholders.
Verify that corresponding entries exist in the maintained locale files (
messages_en.propertiesandmessages_pl.properties).webapp-resources/messages_de.properties (1)
3457-3463: Adding sidebar localization keys to non-maintained German locale.Lines 3457–3463 add seven new localization keys for the System Settings sidebar feature. As this is the German locale file (
messages_de.properties), which is not actively maintained per project standards, the values are provided in English rather than German translations.Per the project learnings, non-maintained locales may remain in English, with AI translation handled in the future. These key additions are consistent with the sidebar refactoring described in the PR objectives.
webapp-resources/messages_ru.properties (1)
3608-3608: No duplicate key found.validate.unsupportedDataTypeis defined only once at line 3608.webapp-resources/messages_nl.properties (1)
3575-3581: English placeholders used in non-maintained Dutch locale.Lines 3575-3581 add new localization keys for the system settings sidebar feature with English text rather than Dutch translations. Per your established practice, non-maintained locales (like Dutch) may use English placeholders pending future AI translation. This approach is consistent with the file's existing patterns and aligns with learnings from prior reviews—no translation is required to proceed.
webapp-resources/messages_lu.properties (1)
3474-3479: Sidebar translation keys added correctly and consistent with project conventions.The six new keys for the system settings sidebar feature (ariaLabel, searchPlaceholder, searchAria, selectAll, clearAll, reset) have been added at the expected line ranges. Per the learnings for this repository, Luxembourgish is a non-maintained locale, and leaving these keys in English is acceptable and does not block the PR. This approach is consistent with the existing mixed English/Luxembourgish content throughout the file.
webapp-resources/messages_zh.properties (1)
3560-3566: Approved. Non-maintained locale — English values are acceptable per project standards.This Chinese (zh) locale is not actively maintained by SCADA-LTS. Per project learnings, English placeholder values in non-maintained locales are acceptable temporarily, with AI translation deferred to a future date. The six new
systemSettings.sidebar.*keys follow the established naming convention and are syntactically correct.webapp-resources/messages_fi.properties (1)
3585-3591: New sidebar translation keys added (English text acceptable for non-maintained locale).Lines 3585–3591 introduce six new sidebar-related translation keys for the system settings sidebar UI. Per the learnings, Finnish is a non-maintained locale, and English text is appropriate for now. The keys follow the naming convention and align with the PR's sidebar feature (search, select all/clear all, reset, and accessibility labels).
| systemSettings.sidebar.ariaLabel=System settings navigation | ||
| systemSettings.sidebar.searchPlaceholder=Search for settings | ||
| systemSettings.sidebar.searchAria=Search settings | ||
| systemSettings.sidebar.selectAll=Select all | ||
| systemSettings.sidebar.clearAll=Clear all | ||
| systemSettings.sidebar.reset=Reset No newline at end of file |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the file size and read the relevant sections
wc -l webapp-resources/messages_pl.propertiesRepository: SCADA-LTS/Scada-LTS
Length of output: 107
🏁 Script executed:
# Read lines around 3598-3603 to confirm the new sidebar keys
sed -n '3595,3610p' webapp-resources/messages_pl.properties
# Check the cited examples at lines 1289 and 1749
sed -n '1288,1290p' webapp-resources/messages_pl.properties
sed -n '1748,1750p' webapp-resources/messages_pl.properties
# Get a broader sense of translation patterns - check some other systemSettings entries
rg 'systemSettings\.' webapp-resources/messages_pl.properties | head -20Repository: SCADA-LTS/Scada-LTS
Length of output: 1718
🏁 Script executed:
# Check the exact lines 3598-3603 with cat to see formatting issues
sed -n '3595,3605p' webapp-resources/messages_pl.properties | cat -A
# Search for other aria labels, placeholders, and button text in the Polish file
rg '(aria|Placeholder|Select|Clear|Reset)' webapp-resources/messages_pl.properties
# Check if similar UI strings elsewhere in Polish file are translated
rg '\.button\.|\.label\.|\.placeholder' webapp-resources/messages_pl.properties | head -10Repository: SCADA-LTS/Scada-LTS
Length of output: 1707
🏁 Script executed:
# Get exact hex dump of the problematic area to see formatting clearly
sed -n '3603,3615p' webapp-resources/messages_pl.properties | od -c | head -50
# Also check if the entries are truly duplicated
rg -c 'systemSettings.sidebar.ariaLabel' webapp-resources/messages_pl.properties
rg -c 'systemSettings.sidebar.reset' webapp-resources/messages_pl.properties
# Get the complete context around line 3603
sed -n '3600,3620p' webapp-resources/messages_pl.propertiesRepository: SCADA-LTS/Scada-LTS
Length of output: 295
🏁 Script executed:
# Get the exact six lines 3598-3603
sed -n '3598,3603p' webapp-resources/messages_pl.properties
# Check how many systemSettings entries total exist in Polish file
rg 'systemSettings\.' webapp-resources/messages_pl.properties | wc -l
# Sample some systemSettings entries to understand the translation pattern
rg 'systemSettings\.[a-zA-Z]+=' webapp-resources/messages_pl.properties | head -30Repository: SCADA-LTS/Scada-LTS
Length of output: 1831
🏁 Script executed:
# Read the raw file content directly without sed to verify Reset109
head -c 100000 webapp-resources/messages_pl.properties | tail -c 2000
# Also check git status and diff to see what was actually added in this PR
git diff HEAD -- webapp-resources/messages_pl.properties | tail -100Repository: SCADA-LTS/Scada-LTS
Length of output: 2063
🏁 Script executed:
# Get the last 20 lines of the file
tail -20 webapp-resources/messages_pl.properties
# Check git diff for this specific file
git diff HEAD -- webapp-resources/messages_pl.properties | head -150Repository: SCADA-LTS/Scada-LTS
Length of output: 1489
🏁 Script executed:
# Get the git diff - the -U context will show what was added/removed
git show HEAD:webapp-resources/messages_pl.properties | tail -20Repository: SCADA-LTS/Scada-LTS
Length of output: 1489
🏁 Script executed:
# Check if messages_en.properties has these sidebar keys
grep 'systemSettings.sidebar' webapp-resources/messages_en.properties
# Check a Polish-adjacent file to understand the pattern
grep -l 'messages.*\.properties' <<< "$(git ls-files webapp-resources/messages_*.properties)" | head -5
# Check messages_de.properties to see if sidebar keys are in German
grep 'systemSettings.sidebar' webapp-resources/messages_de.propertiesRepository: SCADA-LTS/Scada-LTS
Length of output: 663
Add Polish translations for the new sidebar localization keys.
The six new systemSettings.sidebar.* keys are added with English values in the Polish locale file (messages_pl.properties). Since SCADA-LTS actively maintains Polish translations, these user-facing UI strings should be translated to Polish rather than left as English fallbacks.
The Polish locale file contains mixed English and Polish entries. Some systemSettings keys are translated to Polish (e.g., line 1289: header.systemSettings=Ustawienia systemowe), while others remain in English. For consistency with actively-maintained Polish localization, the new sidebar keys should include Polish translations:
systemSettings.sidebar.ariaLabelsystemSettings.sidebar.searchPlaceholdersystemSettings.sidebar.searchAriasystemSettings.sidebar.selectAllsystemSettings.sidebar.clearAllsystemSettings.sidebar.reset
🤖 Prompt for AI Agents
In @webapp-resources/messages_pl.properties around lines 3598 - 3603, The new
sidebar localization keys (systemSettings.sidebar.ariaLabel,
systemSettings.sidebar.searchPlaceholder, systemSettings.sidebar.searchAria,
systemSettings.sidebar.selectAll, systemSettings.sidebar.clearAll,
systemSettings.sidebar.reset) are left in English in messages_pl.properties;
replace their values with Polish translations for consistency (suggestions:
ariaLabel -> "Nawigacja ustawień systemowych", searchPlaceholder -> "Szukaj
ustawień", searchAria -> "Wyszukaj ustawienia", selectAll -> "Zaznacz wszystko",
clearAll -> "Wyczyść wszystko", reset -> "Resetuj") so the Polish locale no
longer falls back to English.
| #sltsFooter{ | ||
| text-align: center; | ||
| position: fixed; | ||
| left: 260px; | ||
| right: 0; | ||
| bottom: 0; | ||
| z-index: 1000; | ||
| height: 24px; | ||
| display: flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
| padding: 0 8px; | ||
| background: #fff; | ||
| border-top: 1px solid #e5e5e5; | ||
| pointer-events: none; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the file exists and read the specific lines
head -n 270 WebContent/assets/common_DARK.css | tail -n 30Repository: SCADA-LTS/Scada-LTS
Length of output: 567
🏁 Script executed:
# Check if there's a light theme version
ls -la WebContent/assets/common*.cssRepository: SCADA-LTS/Scada-LTS
Length of output: 319
🏁 Script executed:
# Search for the CSS variables mentioned in the proposed fix
rg "slts-color-background|slts-color-secondary-lighter" WebContent/Repository: SCADA-LTS/Scada-LTS
Length of output: 2478
🏁 Script executed:
# Look for sidebar width references
rg "260px|sidebar" WebContent/assets/Repository: SCADA-LTS/Scada-LTS
Length of output: 110
🏁 Script executed:
# Check for media queries in the CSS file
rg "@media" WebContent/assets/common_DARK.cssRepository: SCADA-LTS/Scada-LTS
Length of output: 101
🏁 Script executed:
# Check what's in the existing media query at max-width: 730px
rg -A 10 "@media screen and \(max-width: 730px\)" WebContent/assets/common_DARK.cssRepository: SCADA-LTS/Scada-LTS
Length of output: 213
🏁 Script executed:
# Check the light theme CSS for the footer styling
rg -A 10 "#sltsFooter" WebContent/assets/common_DEFAULT.cssRepository: SCADA-LTS/Scada-LTS
Length of output: 242
🏁 Script executed:
# Look for sidebar related styling to confirm the 260px connection
rg "sidebar|slts-sidebar|260" WebContent/assets/ --type cssRepository: SCADA-LTS/Scada-LTS
Length of output: 110
🏁 Script executed:
# Check the entire footer section in DEFAULT CSS to understand theme handling
sed -n '200,300p' WebContent/assets/common_DEFAULT.css | grep -A 20 "#sltsFooter"Repository: SCADA-LTS/Scada-LTS
Length of output: 422
🏁 Script executed:
# Check if there's JavaScript that handles sidebar width dynamically
find WebContent -name "*.js" | head -10 | xargs grep -l "sidebar\|260" 2>/dev/null || echo "No immediate matches"Repository: SCADA-LTS/Scada-LTS
Length of output: 83
🏁 Script executed:
# Let's see the full footer context in DARK and DEFAULT to understand the complete picture
echo "=== DARK THEME ===" && rg -B 5 -A 20 "#sltsFooter" WebContent/assets/common_DARK.css
echo "=== DEFAULT THEME ===" && rg -B 5 -A 20 "#sltsFooter" WebContent/assets/common_DEFAULT.cssRepository: SCADA-LTS/Scada-LTS
Length of output: 899
🏁 Script executed:
# Check if there's a MODERN theme footer as well
rg -A 15 "#sltsFooter" WebContent/assets/common_MODERN.cssRepository: SCADA-LTS/Scada-LTS
Length of output: 45
Hard-coded left: 260px and light colors in DARK theme need fixes for consistency and theming.
The DARK theme's footer has left: 260px with hard-coded background: #fff and border-top: 1px solid #e5e5e5. However, the DEFAULT theme uses left: 0, revealing inconsistency. More critically, white background and light borders conflict with the dark theme and will cause contrast issues. The codebase already establishes the pattern of using theme variables (--slts-color-background, --slts-color-secondary-lighter) throughout both theme files—the footer should follow this pattern. Additionally, a responsive override is needed for narrower viewports, as the 260px offset is only appropriate when the sidebar is visible.
Proposed adjustments
#sltsFooter{
text-align: center;
position: fixed;
left: 260px;
right: 0;
bottom: 0;
z-index: 1000;
height: 24px;
display: flex;
align-items: center;
justify-content: center;
padding: 0 8px;
- background: #fff;
- border-top: 1px solid #e5e5e5;
+ background: var(--slts-color-background);
+ border-top: 1px solid var(--slts-color-secondary-lighter);
pointer-events: none;
}
+
+@media screen and (max-width: 730px) {
+ #sltsFooter {
+ left: 0;
+ }
+}🤖 Prompt for AI Agents
In @WebContent/assets/common_DARK.css around lines 254 - 269, #sltsFooter uses a
hard-coded left: 260px and light color values that break the DARK theme; replace
the fixed values with theme variables (use --slts-color-background for
background and --slts-color-secondary-lighter for the border) and mirror the
DEFAULT layout by defaulting left to 0 (or to a sidebar width CSS variable if
one exists, e.g., --slts-sidebar-width) so it’s consistent across themes; add a
responsive override (media query) to set left: 0 on narrow viewports or when the
sidebar is hidden/collapsed so the footer doesn’t stay offset on small screens.
| #sltsFooter{ | ||
| text-align: center; | ||
| position: fixed; | ||
| left: 0; | ||
| right: 0; | ||
| bottom: 0; | ||
| z-index: 1000; | ||
| height: 24px; | ||
| display: flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
| padding: 0 8px; | ||
| background: #fff; | ||
| border-top: 1px solid #e5e5e5; | ||
| pointer-events: none; | ||
| } |
There was a problem hiding this comment.
Fixed footer may overlap content (and pointer-events: none can cause click-through surprises).
With position: fixed + height: 24px, the last rows/buttons can end up under the footer unless the layout reserves bottom space. pointer-events: none prevents the footer from blocking clicks, but it also means any visible footer area is effectively “click-through”, which can be unexpected.
Proposed adjustment (reserve space; keep footer interactive)
#sltsContent {
padding-top: 10px;
}
#sltsFooter{
text-align: center;
position: fixed;
left: 0;
right: 0;
bottom: 0;
z-index: 1000;
height: 24px;
display: flex;
align-items: center;
justify-content: center;
padding: 0 8px;
background: #fff;
border-top: 1px solid #e5e5e5;
- pointer-events: none;
+ pointer-events: auto;
}
+
+/* Ensure content isn't hidden behind the fixed footer */
+#sltsContent {
+ padding-bottom: 24px;
+}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @WebContent/assets/common_DEFAULT.css around lines 262 - 277, The fixed
footer (#sltsFooter) can overlap page content and pointer-events: none makes it
non-interactive and causes click-through surprises; update styles to reserve
space and keep it interactive by removing pointer-events: none and ensuring the
layout accounts for the footer height (e.g., add body or main content
padding-bottom equal to the footer height, or switch #sltsFooter to position:
sticky at bottom) so the footer no longer covers actionable elements and remains
clickable.
Summary by CodeRabbit
Style
New Features
✏️ Tip: You can customize this high-level summary in your review settings.