Skip to content

luci-theme-openwrt-2020: minor fixes #8964

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.

Sign up for GitHub

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

Open
mcprat wants to merge 2 commits into
base: master
Choose a base branch
from
Open

luci-theme-openwrt-2020: minor fixes #8964

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3bb573c
treewide: fix textarea double scrolling issue
mcprat Aug 22, 2026
60e8555
luci-theme-openwrt-2020: use lighter color for checkboxes
mcprat Aug 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions applications/luci-app-antiblock/htdocs/luci-static/resources/view/antiblock/log.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ return view.extend({
if (element) {
const log = await this.retrieveLog();
element.value = log.value;
element.rows = log.rows;
element.rows = log.rows + 1;
}
},

Expand Down Expand Up @@ -78,7 +78,7 @@ return view.extend({
'style': 'font-size:12px',
'readonly': 'readonly',
'wrap': 'off',
'rows': loglines.rows
'rows': loglines.rows + 1
}, [loglines.value]),
E('div', { 'style': 'padding-top: 20px' }, [scrollUpButton])
])
Expand Down
4 changes: 2 additions & 2 deletions applications/luci-app-antiblock/htdocs/luci-static/resources/view/antiblock/statistics.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ return view.extend({
if (element) {
const log = await this.retrieveLog();
element.value = log.value;
element.rows = log.rows;
element.rows = log.rows + 1;
}
},

Expand Down Expand Up @@ -61,7 +61,7 @@ return view.extend({
'style': 'font-size:12px',
'readonly': 'readonly',
'wrap': 'off',
'rows': loglines.rows
'rows': loglines.rows + 1
}, [loglines.value])
])
]);
Expand Down
4 changes: 2 additions & 2 deletions applications/luci-app-aria2/htdocs/luci-static/resources/view/aria2/files.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ return view.extend({

res.file = res.file || '';
res.content = 'content' in res ? res.content.trim() : '';
res.rows = res.content.split('\n', 20).length;
res.rows = res.content.split('\n', 20).length + 1;
Comment thread
mcprat marked this conversation as resolved.
return res;
})
);
Expand All @@ -35,7 +35,7 @@ return view.extend({
'style': 'width: 100%',
'readonly': true,
'wrap': 'off',
'rows': data.rows >= 20 ? 20 : data.rows + 1
'rows': data.rows >= 20 ? data.rows + 1 : 20

@openwrt-ai openwrt-ai Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flipping the ternary makes every short file render a 20-row box. The commit message reads the original as "reversed logic", but the 20 in res.content.split('\n', 20) at line 20`` is what shows it was a maximum: the producer deliberately stops counting at 20 because the consumer never wanted more than 20 rows. Capping the count only makes sense against a cap.

Concretely, with res.rows now being min(lines, 20) + 1:

  • empty or 1-line aria2.conf — res.rows is 2, 2 >= 20 is false, so the box is 20 rows tall for one line of content. Before this PR it was 2.
  • 19 lines — res.rows is 20, so the >= 20 branch fires one line early and yields 21.
  • 20+ lines — res.rows saturates at 21, so data.rows + 1 is 22 no matter whether the file has 20 lines or 5000. So the "exact number + 1" branch never actually tracks the line count; the cap is still in force, just at 22 now. A genuine "minimum 20, exact count above that" would also have to drop the 20 limit from the split().

Neither effect is mentioned in the commit body, and neither relates to the double-scrolling bug this commit is about. If the intent was to keep the cap and just add the padding row, this restores that and stays consistent with the + 1 already applied at line 20:

Suggested change
'rows': data.rows >= 20 ? data.rows + 1 : 20
'rows': Math.min(data.rows, 21)

If a 20-row minimum really is wanted for the aria2 views, that is a separate UI change and worth its own commit rather than riding along in a treewide scrolling fix.

Generated by Claude Code

@mcprat mcprat Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably an intermediate commit would be best to change the behavior

}, data.content)
)
]);
Expand Down
2 changes: 1 addition & 1 deletion applications/luci-app-package-manager/htdocs/luci-static/resources/view/package-manager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -981,7 +981,7 @@ function handleConfig(ev)
body.push(E('h5', {}, '%h'.format(file)));
body.push(E('textarea', {
'name': file,
'rows': Math.max(Math.min(L.toArray(conf[file].match(/\n/g)).length, 10), 3)
'rows': Math.max(Math.min(L.toArray(conf[file].match(/\n/g)).length, 10), 3) + 1
}, '%h'.format(conf[file])));
});

Expand Down
4 changes: 2 additions & 2 deletions modules/luci-base/htdocs/luci-static/resources/tools/views.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ var CBILogreadBox = function(logtag, name) {
if (element) {
const log = await this.retrieveLog();
element.value = log?.value;
element.rows = log?.rows;
element.rows = log?.rows + 1;

@openwrt-ai openwrt-ai Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

retrieveLog() already returns the padded value — rows: loglines?.length + 1 at views.js:126. So log?.rows is already lines + 1, and this hunk makes the syslog box lines + 2. The commit message says "add 1 to the dynamic size", not 2, so this looks unintended — the + 1 the message notes "already exists in some instances of similar Javascript files" is this same producer-side one, not a separate concern.

Suggested change
element.rows = log?.rows + 1;
element.rows = log?.rows;

The identical double-apply is in the four other log views, each of which already pads at the producer:

producer (already + 1) sites this PR adds a second + 1
views.js:126 this line, and line 280
dmesg.js:109 dmesg.js:122, dmesg.js:287
antiblock/log.js:16`` log.js:28, log.js:81
antiblock/statistics.js:16`` statistics.js:28, statistics.js:64

If lines + 2 really is what fixed the scrolling for you in testing, then the fix belongs in the one producer expression per file rather than in the two consumers, and the commit message should say +2.

Generated by Claude Code

@mcprat mcprat Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is on purpose, to encourage this behavior no matter what example is used for a new application

}
},

Expand Down Expand Up @@ -277,7 +277,7 @@ var CBILogreadBox = function(logtag, name) {
'style': 'font-size:12px',
'readonly': 'readonly',
'wrap': 'off',
'rows': loglines?.rows,
'rows': loglines?.rows + 1
}, [ loglines?.value ]),
E('div', {'style': 'padding-bottom: 20px'}, [scrollUpButton])
])
Expand Down
2 changes: 1 addition & 1 deletion modules/luci-base/htdocs/luci-static/resources/ui.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,7 @@ const UITextarea = UIElement.extend(/** @lends LuCI.ui.Textarea.prototype */ {
'placeholder': this.options.placeholder,
'style': style,
'cols': this.options.cols,
'rows': this.options.rows,
'rows': this.options.rows ? this.options.rows + 1 : this.options.rows,
'wrap': this.options.wrap ? 'soft' : 'off'
}, [ value ]));

Expand Down
4 changes: 2 additions & 2 deletions modules/luci-mod-status/htdocs/luci-static/resources/view/status/dmesg.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ return view.extend({
if (element) {
const log = await this.retrieveLog();
element.value = log.value;
element.rows = log.rows;
element.rows = log.rows + 1;
}
},

Expand Down Expand Up @@ -284,7 +284,7 @@ return view.extend({
'style': 'font-size:12px',
'readonly': 'readonly',
'wrap': 'off',
'rows': loglines.rows
'rows': loglines.rows + 1
}, [ loglines.value ]),
E('div', {'style': 'padding-bottom: 20px'}, [scrollUpButton])
])
Expand Down
2 changes: 1 addition & 1 deletion themes/luci-theme-openwrt-2020/htdocs/luci-static/openwrt2020/cascade.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -981,7 +981,7 @@ button[disabled], button.disabled, .btn[disabled], .btn.disabled {
input[type="checkbox"],
input[type="radio"] {
--bd-color: var(--main-dark-color);
--fg-color: var(--main-dark-color);
--fg-color: var(--main-bright-color);

@openwrt-ai openwrt-ai Aug 23, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This lowers the checked-state contrast in the theme's own (light) rendering. --fg-color paints the checkmark glyph via background: var(--fg-color) on :checked::after at line 1034, and the page background is white ([`body { background: var(--secondary-bright-color) }`](https://github.com/openwrt/luci/blob/500895691b08c46ab9da396a3ea3e8e4f1b0fbbd/themes/luci-theme-openwrt-2020/htdocs/luci-static/openwrt2020/cascade.css#L50-L51),`` #FFFFFF). #00B5E2 on white is roughly 2.4:1, under the 3:1 that WCAG 2.1 SC 1.4.11 asks for on non-text UI components; the current --main-dark-color (#002B49) is roughly 14.6:1.

This theme ships no prefers-color-scheme handling, so the only rendering it controls is the light one — and the commit message says the motivation is a third-party extension that inverts the page. Is trading the default rendering for that the intent? A @media (prefers-color-scheme: dark) override of --fg-color, or brightening only the ::before box outline and leaving the glyph dark, would fix the dark case without regressing the light one.

Generated by Claude Code

@mcprat mcprat Aug 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we should consider changing the color because of WCAG 2.1 SC 1.4.11, but thats for another day...

-webkit-appearance: none; /* nonstandard, should remove in future */
appearance: none;
width: 1em;
Expand Down