Skip to content

Improve DHCP static leases interface (alternative)#3766

Open
rdwebdesign wants to merge 38 commits into
developmentfrom
new/simple-dhcp-static-leases2
Open

Improve DHCP static leases interface (alternative)#3766
rdwebdesign wants to merge 38 commits into
developmentfrom
new/simple-dhcp-static-leases2

Conversation

@rdwebdesign
Copy link
Copy Markdown
Member

@rdwebdesign rdwebdesign commented Apr 22, 2026

What does this PR aim to accomplish?

This is an alternative attempt to create a better user interface to edit Static DHCP leases.

This version is based on #3565 and all previous commits were kept, to avoid losing authorship credits.

Main differences found on this version:

  • the "hint" is shown below the table, instead of on a table row;
  • the textarea can't be changed before all changes made on the table are finished, to avoid losing changes;
  • save button is shown only when needed;
  • button tooltips are shown;
  • only invalid cells are highlighted (no more green cells);
  • validation is now enforced for hostnames when saving (like other values);
  • IPv6 must be enclosed in brackets;
  • a few style adjustements.
New_Static_DHCP_Leases_2

By submitting this pull request, I confirm the following:

  1. I have read and understood the contributors guide, as well as this entire template. I understand which branch to base my commits and Pull Requests against.
  2. I have commented my proposed changes within the code and I have tested my changes.
  3. I am willing to help maintain this change if there are issues with it later.
  4. It is compatible with the EUPL 1.2 license
  5. I have squashed any insignificant commits. (git rebase)
  6. I have checked that another pull request for this purpose does not exist.
  7. I have considered, and confirmed that this submission will be valuable to others.
  8. I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  9. I give this submission freely, and claim no ownership to its content.

  • I have read the above and my PR is ready for review. Check this box to confirm

DL6ER and others added 21 commits July 15, 2025 11:55
…xtarea below

Signed-off-by: DL6ER <dl6er@dl6er.de>
Signed-off-by: DL6ER <dl6er@dl6er.de>
Signed-off-by: DL6ER <dl6er@dl6er.de>
Signed-off-by: DL6ER <dl6er@dl6er.de>
…t any possibility for code injection)

Signed-off-by: DL6ER <dl6er@dl6er.de>
Signed-off-by: DL6ER <dl6er@dl6er.de>
Signed-off-by: DL6ER <dl6er@dl6er.de>
Signed-off-by: DL6ER <dl6er@dl6er.de>
Signed-off-by: DL6ER <dl6er@dl6er.de>
…hemes

Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
Signed-off-by: DL6ER <dl6er@dl6er.de>
Signed-off-by: DL6ER <dl6er@dl6er.de>
Signed-off-by: DL6ER <dl6er@dl6er.de>
…ew feedback

Resolve merge conflict in style/pi-hole.css (keep both StaticDHCPTable
styles and DNSSEC query log styles).

Address outstanding reviewer feedback:
- Change save button icon from floppy-disk to checkmark to clarify it
  confirms the row edit, not a final save
- Update hint text to mention "Save & Apply" is still needed
- Add hostname validation on the hostname cell (rejects spaces, commas,
  and other characters invalid in DNS names)

Signed-off-by: Dominik <dl6er@dl6er.de>
When using the `v` flag, hyphens need to be escaped inside a character class.

Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
Fix regex adding `v` flag and fix line breaks to respect maximum lenght

Note: these issues were not previously reported because there was a
previous merge conflict. The prettier test was executed only after the
merge conflict was resolved.

Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign rdwebdesign requested a review from a team as a code owner April 22, 2026 21:18
- only show save button when needed
- use fixed size and right aligned text for button column
- highlight cells only on error and remove the highlight when is fixed
- replace a few inline CSS styles with proper HTML tags
- adjust CSS on specific themes

Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
- replace DOMContentLoaded listener with a function and call it when needed
- use javascript to update the CSS variable `--num-lines`
- use CSS to calculate the elements height

Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign rdwebdesign force-pushed the new/simple-dhcp-static-leases2 branch 4 times, most recently from bf69448 to 8ba9c79 Compare April 22, 2026 22:09
- move and group some CSS selectors in the file, to make editing easier
- use semi-transparent background color for .table-danger
- reduce specificity for StaticDHCPTable cells and .line-numbers
- remove unused and duplicated selectors

Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
…data

If enabled, the textarea can be edited, which triggers a table rewrite,
resulting in the loss of all unsaved changes made to the table rows.

This commit keeps the textarea disabled until all edited rows are saved.

Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign rdwebdesign force-pushed the new/simple-dhcp-static-leases2 branch from 0d919cf to ceddab9 Compare April 25, 2026 04:15
When editing a row, two new buttons will be visible: Confirm and Cancel

Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
The intention here is to improve readability

Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign rdwebdesign force-pushed the new/simple-dhcp-static-leases2 branch from ceddab9 to 8c6bf7b Compare April 29, 2026 22:07
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@yubiuser
Copy link
Copy Markdown
Member

yubiuser commented May 5, 2026

This looks really nice now.
I noticed, that we do a IP validation when entering via the table field, but not when entered via the text area. Invalid IPs won't be shown in the table (with a red background) but are completely empty. I wonder if we should show it but give it the red background instead?

@rdwebdesign
Copy link
Copy Markdown
Member Author

rdwebdesign commented May 5, 2026

Hmmmm... What did you type in your test?

I'm not sure if we can validate that in every case.

The issue here is: we don't have a fixed number of items to make sure the second one is always an IP.

On the table, we can always say the second column should be an IP, but when the text is typed on the textarea, the second item can be a hostname... or an invalid IP, or many other options.
How to tell them apart?

@yubiuser
Copy link
Copy Markdown
Member

yubiuser commented May 5, 2026

You are correct - we can't know that the second item in from the textarea is an invalid IP when it could also be something different (id, tag). Scratch my idea.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR provides an alternative UX for managing static DHCP leases by introducing an editable table view that stays in sync with the underlying dhcp.hosts textarea, while improving validation, tooltips, and styling across themes.

Changes:

  • Added a dedicated “Static DHCP” table UI with per-row confirm/cancel behavior and synchronized textarea + line numbers.
  • Expanded client-side validators (IPv4/IPv6 helpers, bracketed IPv6, stricter hostname checks) and applied them to table editing.
  • Updated core/theme CSS to support the new layout, hint messaging, and validation highlighting.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
style/themes/lcars.css Theme-specific tweaks for button sizing and the new static DHCP hint/disabled-textarea tooltip styles.
style/themes/high-contrast.css Adds StaticDHCPTable-specific styling hooks for danger/success highlighting.
style/themes/high-contrast-dark.css Adds StaticDHCPTable-specific styling hooks for danger/success highlighting.
style/pi-hole.css Adds layout/UX CSS for the StaticDHCPTable, hint message, textarea wrapper, and line numbers.
settings-dhcp.lp Reworks the static DHCP section layout: introduces the table UI, advanced notes, examples cards, and textarea wrapper with line numbers.
scripts/js/utils.js Adds IPv4/IPv6 convenience validators, bracketed IPv6 validation, stricter hostname validation, and updates MAC/hostname trimming behavior.
scripts/js/settings-dhcp.js Implements the static DHCP table rendering/editing workflow, per-row save/cancel, validation highlighting, tooltips, and textarea line numbering.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scripts/js/settings-dhcp.js
Comment thread scripts/js/settings-dhcp.js Outdated
Comment thread scripts/js/utils.js Outdated
Comment thread scripts/js/utils.js Outdated
Comment thread settings-dhcp.lp Outdated
@rdwebdesign rdwebdesign force-pushed the new/simple-dhcp-static-leases2 branch 2 times, most recently from c877479 to c574020 Compare May 6, 2026 01:15
This function tries to use a simplified version of the dnsmasq code used
to parse dhcp-host options.

Also, use the parse function when we cancel edit operation on a line

Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign rdwebdesign force-pushed the new/simple-dhcp-static-leases2 branch from c574020 to 62132c8 Compare May 6, 2026 01:44
Also fix 2 issues on validation functions:
- better regex for validateMAC()
- make sure validateIPv6Brackets() correctly trims the value before
  validation

Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign rdwebdesign force-pushed the new/simple-dhcp-static-leases2 branch from 09a7fcc to e6fe47c Compare May 6, 2026 03:09
@rdwebdesign
Copy link
Copy Markdown
Member Author

@yubiuser

I think the new parse function will solve some issues, like when "infinite" was considered as hostname (Example: aa:bb:cc:dd:ee:ff, 192.168.0.2, infinite). Now "infinite" is always considered as a lease time string (like on dnsmasq code).

Also, now the position of the items are not really important. The function will parse them and, if valid, will show them on the table, in the correct columns.

…ping

Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
Now we allow invalid values on the table and highlight them.
This will allow users to find errors in data already saved on pihole.toml.

New parsing rules added - The line is also considered as "advanced" if:
  - the line contains an asterisk;
  - more than one IP is found;
  - more than one Mac Address is found;
  - more than one hostname is found.

The table also highlights invalid data onload, before a row is clicked.

Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign rdwebdesign force-pushed the new/simple-dhcp-static-leases2 branch from 74af516 to a88b579 Compare May 8, 2026 00:29
@rdwebdesign rdwebdesign requested review from PromoFaux and removed request for DL6ER May 19, 2026 17:09
@github-actions
Copy link
Copy Markdown
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@github-actions
Copy link
Copy Markdown
Contributor

Conflicts have been resolved.

Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign rdwebdesign force-pushed the new/simple-dhcp-static-leases2 branch from 6c8f16c to 5eef250 Compare May 19, 2026 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants