Skip to content

Better user interface to edit reverse DNS servers (dns.revServers)#3769

Open
rdwebdesign wants to merge 7 commits into
developmentfrom
tweak/multi_revServers2
Open

Better user interface to edit reverse DNS servers (dns.revServers)#3769
rdwebdesign wants to merge 7 commits into
developmentfrom
tweak/multi_revServers2

Conversation

@rdwebdesign

@rdwebdesign rdwebdesign commented May 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

What does this PR aim to accomplish?

Replace the current single <textarea> with a table.
Each reverse server will be in a separate row and each field will be in a separate table cell.

A small legend will improve usability (maybe we will need to change the help text).

This is an update of #2943

How does this PR accomplish the above?

Using datatables plugin and the FTL API to save dns.revServers entries immediately.

image

This is still a work in progress.

TODO:

  • add validation function to the text fields
  • fix waitMe calls
  • test properly
  • check the visual on all themes

@rdwebdesign rdwebdesign self-assigned this May 1, 2026
@rdwebdesign rdwebdesign requested a review from a team as a code owner May 1, 2026 04:35
@rdwebdesign rdwebdesign force-pushed the tweak/multi_revServers2 branch 5 times, most recently from 6233bee to 13b1af4 Compare May 1, 2026 05:06
github-advanced-security[bot]
github-advanced-security AI found potential problems May 1, 2026
View reviewed changes
Comment thread scripts/js/utils.js Fixed
@rdwebdesign rdwebdesign force-pushed the tweak/multi_revServers2 branch 2 times, most recently from b55efd8 to 7574a91 Compare May 1, 2026 05:42
yubiuser
yubiuser requested changes May 1, 2026
View reviewed changes

@yubiuser yubiuser left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Saving gives an error about waitME

Image

I think the re-filled example should not be ticked enabled by default if there is already an entry.

Image

If you input a wrong network range and click add, an error 400 bad request will be shown in the console. Then the whole input areas are cleaned. I think it would be nicer if the wrong values would be preserved so users don't need to input everything but can just fix the issue.
Additionally, the wrong input will still be present in the payload, so FTL will reject it

Image

@rdwebdesign

rdwebdesign commented May 1, 2026

Copy link
Copy Markdown
Member Author

Saving gives an error about waitME

Yeah... I think I messed up the waitME. I will take a look.

I think the re-filled example should not be ticked enabled by default if there is already an entry.

We can change that.
I just thought that users might want to enable the most recently added line.

If you input a wrong network range and click add, an error 400 bad request will be shown in the console.
...
Additionally, the wrong input will still be present in the payload, so FTL will reject it.

This is expected at the current PR state.
As I said, this is still a work in progress.
I still need to add validation for all fields and test the changes.

@rdwebdesign

rdwebdesign commented May 1, 2026

Copy link
Copy Markdown
Member Author

@yubiuser

I will mark as draft until I have the time to add validation and fix some issues.

Please, feel free to keep testing the code even before I finish the PR.

@rdwebdesign rdwebdesign marked this pull request as draft May 1, 2026 17:47
@yubiuser

yubiuser commented May 1, 2026

Copy link
Copy Markdown
Member

I just thought that users might want to enable the most recently added line.

Maybe we can enable it automatically once users start to add real input in the fields?

@rdwebdesign rdwebdesign force-pushed the tweak/multi_revServers2 branch from 7574a91 to ccbc24b Compare May 1, 2026 23:13
@rdwebdesign

rdwebdesign commented May 1, 2026

Copy link
Copy Markdown
Member Author

Saving gives an error about waitME

Fixed.

It was a previous bug: you can't call .hideAll() function on the element.

This function was created to be called directly on the class (like WaitMe.hideAll() - notice the uppercase W). I don't think we want to use this function.

I used the .hide() function, intended to hide individual elements. Now it works.

@yubiuser

yubiuser commented May 3, 2026

Copy link
Copy Markdown
Member

Confirm waitMe error is gone.

@rdwebdesign

rdwebdesign commented May 3, 2026

Copy link
Copy Markdown
Member Author

Thanks for testing.

It would be better if we first merge PR3766. I want to use some of the validation functions already added there.

@github-actions

github-actions Bot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

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

@github-actions

github-actions Bot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Conflicts have been resolved.

@rdwebdesign
Use a table to show and edit Reverse DNS Servers
e7b0b77 
- add functions and buttons to add, modify, undo changes and delete lines
- generate the table using data already on the textarea
- add functions to read textarea contents and parse then into an array
- add functions to update the textarea contents from the table
- hide the textarea. Only the table will be visible
- make sure the table is not shown when the option was set by ENV VARS
- move some help paragraphs to the table legend
- remove usused original help text for the textarea
- use fixed size for all action buttons
- don't use "icheck" styling on the checkboxes in the table

Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign
Fix loadingOverlayTimeoutCallback() to actually remove waitMe overlay
f2a2356 
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign rdwebdesign force-pushed the tweak/multi_revServers2 branch from 4507ed9 to c073434 Compare June 13, 2026 19:54
github-advanced-security[bot]
github-advanced-security AI found potential problems Jun 13, 2026
View reviewed changes
Comment thread scripts/js/settings-dns.js Fixed
@rdwebdesign rdwebdesign force-pushed the tweak/multi_revServers2 branch 2 times, most recently from b45bd22 to 3fb4943 Compare June 13, 2026 20:20
@rdwebdesign
Refactor the code to actually save the Conditional Forwarding data
2467c74 
Previously, the same function was used to initialize the table and update
the table rows, when lines were added, edited or deleted.

Now, one function initializes the table and another function saves the
changes, shows a message and updates the table to reflect the changes.

The values are "saved and applied" immediately using an AJAX call.
The waitMe overlay is also shown, but the page is NOT reloaded.

Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign
Remove unnecessary attribute
7797db6 
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign rdwebdesign force-pushed the tweak/multi_revServers2 branch 3 times, most recently from 81637a3 to be13d75 Compare June 13, 2026 20:59
github-advanced-security[bot]
github-advanced-security AI found potential problems Jun 13, 2026
View reviewed changes
Comment thread scripts/js/settings-dns.js Fixed
@rdwebdesign rdwebdesign force-pushed the tweak/multi_revServers2 branch 2 times, most recently from fec8044 to 82f2b7c Compare June 13, 2026 22:08
@rdwebdesign
Add validation to editabled table cells and simplify the table
770a7ca 
- remove `<input>` tags and add contenteditable attribute to all
  editable cells
- rows are always enabled now (no "edit" button to enable them)
- Simplify javascript code:
  - use classNames to simplify column selection, instead of unique ids
  - use datatables "columnDefs" and "createdCell" to add attributes
  - use CSS class "editing" and CSS rules to handle buttons
- remove unused and unnecessary attributes from elements

Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign
Add button title and remove unused attribute
c7f467a 
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign rdwebdesign force-pushed the tweak/multi_revServers2 branch from 82f2b7c to e48dcfe Compare June 14, 2026 00:50
@rdwebdesign rdwebdesign marked this pull request as ready for review June 14, 2026 00:57
@rdwebdesign
Allow to add an optional port to the reverse server IP
d449e01 
and include validation functions

Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign rdwebdesign force-pushed the tweak/multi_revServers2 branch from e48dcfe to d449e01 Compare June 14, 2026 01:02
@rdwebdesign rdwebdesign requested review from a team and yubiuser June 14, 2026 01:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yubiuser yubiuser Awaiting requested review from yubiuser

1 more reviewer

@github-advanced-security github-advanced-security[bot] github-advanced-security[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Requested changes must be addressed to merge this pull request.

Assignees

@rdwebdesign rdwebdesign

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@rdwebdesign @yubiuser @github-advanced-security