pi-hole / web

Pi-hole Dashboard for stats and more
https://pi-hole.net
Other
1.98k stars 549 forks source link

New layout for "All Settings" page #3042

Closed rdwebdesign closed 1 month ago

rdwebdesign commented 1 month ago

What does this PR accomplishes?

This PR creates a cleaner layout for All Settings page, removing a few boxes and borders.

It also improves the number of columns, depending on the screen size (1 column in small and medium screens, 2 columns in large screens and 3 columns in very large screens).


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.

PromoFaux commented 1 month ago

Overall, I like this change - it looks really smart. One possible change request, however... Could we make it more obvious that the tab headers are, in fact, tab headers? Took me a moment to notice that they were there as the background colour of the buttons is the same as the page background - doesn't visually stick out as a tabbed layout (to my eye!)

yubiuser commented 1 month ago

One small drawback with tabs is that you can't search across all settings right away eg. directly jump to a certain debug setting.

PromoFaux commented 1 month ago

Ah yes, I suppose currently you can load the page and ctrl+f

I wonder, as the data is technically still on the page (just hidden with some CSS?) Is there some super magic js/css that can be employed here to make the tab get selected and shown based on the contents of the ctrl+f search string? Or am I wishing for the impossible? I suppose a simple search box on at the top could be easier to wire up...

rdwebdesign commented 1 month ago

Well... with tabs there will be less visible content on each tab, making it easier to visually search, but slightly more difficult to search using CTRL + F (unless you know which tab to look for).

Or am I wishing for the impossible?

Not possible, because display: none really hides the content.

PromoFaux commented 1 month ago

Sorry - having "ideas" again...

When toggling to modified settings only, could we either: a) Add some sort of indication to a tab that contains modified settings b) Remove any tabs that have no modified settings to display

Might save a few clicks when looking for modified settings

Not possible, because display: none really hides the content.

I figured that would be the case. What about a custom search box at the top of the all settings page, above the tabs/next to the toggle switch

DL6ER commented 1 month ago

I do like this a lot. It even keeps adaptivity so when FTL decides at any point to add a new main category, we do not need to change the web code to make it appear:

image

Note: This is a real example when running FTL branch new/ntp