luci-theme-openwrt-2020: minor fixes

[mcprat]

Pull request details

1. scrolling issue for syslog (see commit 24fbd75)
2. more balanced brighter color for checkbox check (visibility for dark mode)

---

Tested on

Web browser(s): Win 10 64-bit Chrome 151.0.7922.169

[mcprat]

ping @jow- @hnyman

[openwrt-ai]

Commit checks

• 5008956 "luci-theme-openwrt-2020: use lighter color for checkboxes" — the subject and body talk about checkboxes only, but the changed declaration sits in the shared input[type="checkbox"], input[type="radio"] block and input[type="radio"]:checked::after also paints with var(--fg-color), so radio buttons are recoloured too. Either mention radios in the message or scope the override to input[type="checkbox"].

---

Generated by Claude Code

[openwrt-ai]

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]

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

[mcprat]

I see that using overflow-y causes other problems elsewhere and got reverted

bumping the "rows" value also fixes the problem while not blocking a scrollbar from appearing in other contexts.

I think this is ready now

[openwrt-ai]

Re-reviewed after the force-push: the CSS overflow-y approach was replaced by a JS rows + 1 approach, so the first commit is entirely new. The previous commit-check on the checkbox commit is addressed — 32970c5 now documents the radio-button side effect.

Commit checks

• db70415 "treewide: fix textarea double scrolling issue" — the body says "add 1 to the dynamic size of the textarea boxes", but in five of the seven files the value being incremented has already been padded by its producer, so the effective change is +2, not +1. The message also reads the pre-existing + 1 as belonging to unrelated files ("this + 1 already exists in some instances of similar Javascript files") when it is in fact the same padding, in the same files this commit touches. Either drop the duplicate hunks or state that +2 is intended. Details inline.

---

Generated by Claude Code

[openwrt-ai]

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

[openwrt-ai]

rows is optional here and defaults to null — rows: null in __init__ at ui.js:511. null + 1 is 1, so every ui.Textarea constructed without an explicit rows now renders a one-line-tall textarea. Before this change the null was dropped by attr[key] == null in dom.attr() at luci.js:1503, so no rows attribute was emitted at all and the theme CSS governed the height.

The form.TextValue path is hit too but lands differently: rows: this.rows at form.js:5356 passes undefined for options that never set .rows, and Object.assign overwrites the null default with that undefined, so those get rows="NaN" — an invalid attribute value the browser discards. Affected in-tree options that never assign .rows include cloudflared config.js:62 and [keepalived peers.js:83](https://github.com/openwrt/luci/blob/32970c53fc94d6659253154812e53cbfee8f2fe0/applications/luci-app-keepalived/htdocs/luci-static/resources/view/keepalived/peers.js#L83),`` plus samba4, ksmbd, ocserv, lldpd, smartdns, clamav and nlbwmon.

Only pad when a row count was actually supplied, so the unset case keeps being omitted:

Suggested change

	'rows': this.options.rows + 1,
	'rows': this.options.rows != null ? this.options.rows + 1 : null,

---

Generated by Claude Code

[openwrt-ai]

Padding here misses the case that actually has the bug and over-pads the one that doesn't. The consumer already adds the row itself in its else-branch — 'rows': data.rows >= 20 ? 20 : data.rows + 1 at files.js:38.``

Because split('\n', 20) caps the array at 20 entries, res.rows maxes out at 20 before this change:

• ≥ 20 lines (the case that double-scrolls): res.rows becomes 21, still >= 20, so the ternary still yields exactly 20 rows for 20 displayed lines. Unchanged — and still buggy.
• ≤ 18 lines: now lines + 2 instead of lines + 1.
• 19 lines: res.rows becomes 20, which now trips the >= 20 branch, so the cap boundary moves one line earlier.

It also makes res.rows no longer mean "number of lines", which is what line 38 reads it as. Revert this line:

Suggested change

	res.rows = res.content.split('\n', 20).length + 1;
	res.rows = res.content.split('\n', 20).length;

and fix the capped branch at line 38 instead — 'rows': Math.min(data.rows, 20) + 1 gives 21 rows for the capped case and keeps lines + 1 everywhere below it.

---

Generated by Claude Code