Skip to content
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.

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

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

Draft
wants to merge 15 commits into
base: development
Choose a base branch
from

Conversation

rdwebdesign
Copy link
Member

@rdwebdesign rdwebdesign commented Jan 27, 2024

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).

NOTE:

Depends on #2885 (already merged)

How does this PR accomplish the above?

Using datatables plugin and a few functions to deal with dns.revServers entries.

image


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

@rdwebdesign rdwebdesign changed the base branch from master to new/multi_revServer January 27, 2024 19:01
@rdwebdesign rdwebdesign force-pushed the tweak/multi_revServers branch 3 times, most recently from 18799fe to 64efa24 Compare January 27, 2024 20:25
@rdwebdesign rdwebdesign requested review from a team and removed request for a team February 2, 2024 19:54
@rdwebdesign rdwebdesign marked this pull request as draft February 3, 2024 03:12
Copy link
Member

@DL6ER DL6ER left a comment

Choose a reason for hiding this comment

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

I did test your PR and it looks really great overall. However, I see you moved this PR into draft mode again and I hope the reason is: because you are adding editing :-)

The first thing I noticed was my currently defined revServer cannot be disabled. The ideal solution would be having <input> boxes in the table and allow full-blown editing (rendering the new multi-line raw value for revServers fromt he fields on save).

Screenshot from 2024-02-03 07-54-11

@rdwebdesign
Copy link
Member Author

The first thing I noticed was my currently defined revServer cannot be disabled.

This is expected... In the current state, this PR can only add and delete entries. I didn't create functions to edit fields (this checkbox is just a nice way to display true/false values).

I hope the reason is: because you are adding editing

This is in my todo list.

@rdwebdesign rdwebdesign force-pushed the tweak/multi_revServers branch 2 times, most recently from 043ce9d to da84675 Compare February 4, 2024 17:56
Base automatically changed from new/multi_revServer to development-v6 February 11, 2024 05:52
Copy link
Contributor

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

Copy link
Contributor

Conflicts have been resolved.

This commit will be removed.

Signed-off-by: RD WebDesign <[email protected]>
Copy link
Contributor

github-actions bot commented Apr 2, 2024

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

Copy link
Contributor

github-actions bot commented Apr 2, 2024

Conflicts have been resolved.

@rdwebdesign rdwebdesign changed the base branch from development-v6 to development September 3, 2024 19:14
@yubiuser
Copy link
Member

@rdwebdesign
How is the state on this PR. Is this something you want to include/hope to finish for v6?

@rdwebdesign
Copy link
Member Author

I completely forgot about it.

I will take a look again when I find some time, but this is not necessary to v6 release.
This is just an improvement and can be done later.

@DL6ER
Copy link
Member

DL6ER commented Sep 25, 2024

I agree, it'd also a nice v6.1 thing

Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants