Add suggested domains in group domains page

[anthony-de-jong]

By submitting this pull request, I confirm the following:

• I have read and understood the contributors guide, as well as this entire template.
• I have made only one major change in my proposed changes.
• I have commented my proposed changes within the code.
• I have tested my proposed changes.
• I am willing to help maintain this change if there are issues with it later.
• I give this submission freely and claim no ownership.
• It is compatible with the EUPL 1.2 license
• I have squashed any insignificant commits. (git rebase)
• I have Signed Off all commits. (git commit --signoff)

---

What does this PR aim to accomplish?:

Make adding domains easier by suggesting (sub-)domains when typing or pasting an URL into the group domains page:

How does this PR accomplish the above?:

Adding a input event listener on the #new_domain input element with a debounce of 1000ms. When executed it checks for a valid URL by passing the #new_domain value into new URL(...). If the value is a valid URL it creates a table with the suggested (sub-)domains. Clicking on a suggested (sub-)domain sets the value of #new_domain to the selected (sub-)domain and hides the table.

What documentation changes (if any) are needed to support this PR?:

None that I'm aware of.

[rdwebdesign]

You can ignore these 2 errors related to IE11:

Error: /home/runner/work/AdminLTE/AdminLTE/scripts/pi-hole/js/groups-domains.js: line 91, col 19, Error - URL.hostname() is not supported in IE 11 (compat/compat)
Error: /home/runner/work/AdminLTE/AdminLTE/scripts/pi-hole/js/groups-domains.js: line 91, col 19, Error - URL is not supported in op_mini all, IE 11 (compat/compat)

Please, verify the other errors.

[yubiuser]

Thanks for your contribution. I really like the idea.
Is there a reason you designed it to only work by pasting and not by entering URLs by hand? With a litte delay after the last keystroke (maybe 1sec) it could also suggest domains if more then one would be possible.

[anthony-de-jong]

I've based it on my most common use case. That is when browsing on a mobile phone encountering a page that is blocked that shouldn't or isn't that should. I copied the whole URL and pasted it into pi-hole's new_domain field. And then I would've needed to re-copy and paste the domain part while struggling with the imprecise selection markers or trying removing the path without holding the backspace key for to long otherwise it would've deleted the whole text. One to many frustrated moments later I just decided to implement it for myself a couple of weeks ago. And this week I had some free time decided to polish it and make a PR. So that's the backstory how it got created and the reason I designed it this way ;)

Is there a reason you designed it to only work by pasting and not by entering URLs by hand?

The thought crossed my mind as well, but I decided against it because it would be silly to type https://tracking.bad.big-coporation.com/analytics.php?id=1377 to only then select a suggested subdomain e.g. .bad.big-coporation.com. Maybe you are talking about a scenario I'm not thinking of?

With a little delay after the last keystroke (maybe 1sec) it could also suggest domains if more then one would be possible.

Do you mean to extend the functionality that allows you to add multiple domain in a single action by separating them with a space? So to first split the new_domain value on a space and to suggest domain based on that?

Or do you mean that in case of https://tracking.bad.big-coporation.com/analytics.php?id=1377 you can select multiple suggested (sub-)domains resulting in the value tracking.bad.big-coporation.com bad.big-coporation.com big-coporation.com that could be added in a single action?

It's totally possible that you're talking about a scenario I'm not seeing. Could you explain the scenario a bit more or give some example values that I'm not thinking of?

As a side node, listening for a paste event can give instant user feedback in comparison to a debounce of 500ms - 1sec when listening on the input event.

[yubiuser]

I totally see where you came from ;-)

What I meant is a lighter version of

The thought crossed my mind as well, but I decided against it because it would be silly to type https://tracking.bad.big-coporation.com/analytics.php?id=1377 to only then select a suggested subdomain e.g. .bad.big-coporation.com.

Silly or not, I've seen people typing https://tracking.bad.big-coporation.com because they don't know the difference between domain, protocol, URL. It would be great if they would be offered a valid domain (and reduce support burden) if they are typing instead of pasting

---

Do you mean to extend the functionality that allows you to add multiple domain in a single action by separating them with a space?

I didn't thought about this so far, but think it is not needed. How do you want to present a large number of domains in a suitable way if users mass input URLs? They will be rejected anyway (contained invalid characters), so users should think about what they input.

---

Or do you mean that in case of https://tracking.bad.big-coporation.com/analytics.php?id=1377 you can select multiple suggested (sub-)domains resulting in the value tracking.bad.big-coporation.com bad.big-coporation.com big-coporation.com that could be added in a single action?

This would also be a nice feature. Make them selectable? From a UI perspective - how do we need to deal with "Add domain as wildcard" then?

[anthony-de-jong]

Silly or not, I've seen people typing https://tracking.bad.big-coporation.com because they don't know the difference between domain, protocol, URL. It would be great if they would be offered a valid domain (and reduce support burden) if they are typing instead of pasting

Fair point. I'll take a look at it next week when I've some spare time. Should be an easy modification.

---

I didn't thought about this so far, but think it is not needed. How do you want to present a large number of domains in a suitable way if users mass input URLs? They will be rejected anyway (contained invalid characters), so users should think about what they input.

Thinking about it some more you could support showing suggested domains when the pasted value is separated with a space from the existing text in new_domain and the pasted value doesn't throw when put into new URL() (and check if it doesn't contain a space because the URL constructor happily convert a space to %20).

---

Or do you mean that in case of https://tracking.bad.big-coporation.com/analytics.php?id=1377 you can select multiple suggested (sub-)domains resulting in the value tracking.bad.big-coporation.com bad.big-coporation.com big-coporation.com that could be added in a single action?

This would also be a nice feature. Make them selectable? From a UI perspective - how do we need to deal with "Add domain as wildcard" then?

I was thinking about only removing the selected suggestion and change the Did you mean to 'Also add'. And when clicking on another suggestion to append it to the new_domain value.
Personally I don't usually add multiple domains per action so I don't know how common the usecase would be in combination with suggested domains.

---

I think that the current version (with typing support) is a great first step. And if there is a need to extend the functionally I'll be happy to work on it some more

[rdwebdesign]

Let's see the simpler version first.
I don't think multiple domains at the same time is common at all.

[yubiuser]

I think that the current version (with typing support) is a great first step. And if there is a need to extend the functionally I'll be happy to work on it some more

I agree. Happy to see your updated version.

[anthony-de-jong]

I've updated the feature to also work when typing.

Do I need to update the name and description of the pull request since it doesn't mention the typing part?

[yubiuser]

Thanks for the addition. On small thing: We need clearance of the suggestions if users change the tab to RegEx and back (same as we clear the input field) - currently the suggestion is still there.

[rdwebdesign]

The code suggested above can be found here:
https://github.com/pi-hole/AdminLTE/blob/1783d260863d5b5d3e00eee2a795f4880d5fd06e/scripts/pi-hole/js/groups-domains.js#L40-L54

[anthony-de-jong]

Thanks for the addition. On small thing: We need clearance of the suggestions if users change the tab to RegEx and back (same as we clear the input field) - currently the suggestion is still there.

Ah, small oversight. I've fixed it and squashed it into 1 commit.

---

Do I need to update the name and description of the pull request since it doesn't mention the typing part?

Do I need to do anything with this?

[yubiuser]

Do I need to do anything with this?

Yes, please. Makes it easier to find relevant PRs if needed in the future.

[yubiuser]

@anthony-de-jong

Thanks for your PR. Please don't be demotivated by the changes we requested, we really appreciated your PR and effort. We hope you'll continue contributing to the project.

[pralor-bot]

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/pi-hole-ftl-v5-16-web-v5-13-and-core-v5-11-1-released/56384/1