Improve DNS upstream table - #3827
Conversation
Signed-off-by: yubiuser <github@yubiuser.dev>
Signed-off-by: yubiuser <github@yubiuser.dev>
There was a problem hiding this comment.
Pull request overview
Fixes the upstream DNS selection UI logic by targeting the correct upstream tables across Plain/DoT/DoH tabs and improving parsing of the custom-upstreams textarea.
Changes:
- Adds a stable wrapper ID (
DNSupstreamTabs) around the upstream tabs to scope DOM queries correctly. - Updates checkbox-change handling to iterate over all upstream tables (plain/dot/doh) instead of a non-existent
#DNSupstreamsTable. - Improves textarea splitting for Windows CRLF and filters out empty lines (with a remaining whitespace-only edge case noted in review).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| settings-dns.lp | Adds id="DNSupstreamTabs" to the upstream tabs container so JS can reliably scope table/checkbox queries. |
| scripts/js/settings-dns.js | Fixes checkbox iteration selector to cover all upstream tables and improves newline parsing of the custom upstreams textarea. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: yubiuser <github@yubiuser.dev>
|
The last commit improves the custom upstream input box to really only show entries that do not match any selected entry from the table above. I always found it confusing that the "custom upstream" box also contained non-custom but selected upstreams. It now keeps both things separate and only merged them before the list of upstreams is send to the API |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (2)
scripts/js/settings-dns.js:12
- Grammar in the comment: “manual entered” should be “manually entered”.
// Get the manual entered upstream servers from the textarea
scripts/js/settings.js:146
- For robustness and consistency with other textarea parsing in this codebase (e.g., settings-dns.js/settings-dhcp.js), consider splitting textarea values with /\r?\n/ instead of "\n" so CRLF inputs don’t leave trailing "\r" characters in array entries.
if ($(this).is("textarea")) {
value = $(this).val();
value = value === "" ? [] : value.split("\n");
// On DNS settings page, upstream table selections are merged at save
// time so the textarea can remain manual-only in the UI.
if (key === "dns.upstreams" && typeof globalThis.getMergedDNSupstreams === "function") {
value = globalThis.getMergedDNSupstreams(value);
… field Signed-off-by: yubiuser <github@yubiuser.dev>
What does this PR aim to accomplish?:
This PR does two things: 1) fix the DNS upstream table 2) improve the custom upstream input box
Fix DNS upstream table.
Main fix:
DNSupstreamsTabledoes not exist, butDNSupstreamsTable-plan,DNSupstreamsTable-dohandDNSupstreamsTable-dot.Additionally, if no title is set (DNS server address), return early.
Additionally 2: also break on Windows CRLF and remove empty entries from the input field.
The last commit improves the custom upstream input box to really only show entries that do not match any selected entry from the table above. I always found it confusing that the "custom upstream" box also contained non-custom but selected upstreams. It now keeps both things separate and only merged them before the list of upstreams is send to the API
By submitting this pull request, I confirm the following:
git rebase)