Skip to content

New layout for "All Settings" page#3042

Merged
rdwebdesign merged 17 commits into
development-v6from
new/settings_all_layout
Jun 6, 2024
Merged

New layout for "All Settings" page#3042
rdwebdesign merged 17 commits into
development-v6from
new/settings_all_layout

Conversation

@rdwebdesign

@rdwebdesign rdwebdesign commented Jun 4, 2024

Copy link
Copy Markdown
Member

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.
  • I have read the above and my PR is ready for review. Check this box to confirm

@rdwebdesign rdwebdesign force-pushed the new/settings_all_layout branch 3 times, most recently from db33e79 to 7c823de Compare June 4, 2024 20:10
rdwebdesign added 13 commits June 4, 2024 17:56
@rdwebdesign
Move code used to format icons to a dedicated function
44e1a9b 
also surround the icons with a SPAN tag.

Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign
Move value details and layout to a dedicated function
e9df006 
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign
Add a few CSS classes
84bd977 
- add class `row` to form-group (because it contains columns) and move
  the tag to the details function
- add class `col-sm-12` to boolean values
- add new class `settings-box` to allow targeting only settings boxes

Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign
Move value.description to box-body and the content to box-footer
03c0d3e 
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign
Replace non-breaking spaces with a margin
494c1cf 
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign
CSS adjustments
d66c5ec 
- change label style and width
- set icons padding

Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign
Use full width for array values
3a5b791 
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign
Use radio buttons for enum types
3e6d0e9 
This replaces the original `<select>` and merges the options with the
"allowed values" block in a single one, simplifying the presentation.
Also, the defaultValueHint was incorporated into the default value.

Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign
More CSS adjustments
f2f6cf4 
- Adjust styles to show only one column in mid and small screens
- also group all "settings" styles

Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign
Cleaner layout for "All Settings" page
f0664ae 
- remove the external green boxes;
- remove border from inner boxes (and change the box-shadow);
- add navigation and tabs for settings sections;
- adjust/add some classes for formatting

Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign
Remove the word "settings" from each tabs button
c202f55 
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign
Show a message for empty tabs when selecting "Only Modified" settings
6e6c667 
and fix the navigation basic style colors

Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign
Change the number of columns depending on the screen size
5db10ae 
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign rdwebdesign force-pushed the new/settings_all_layout branch from 7c823de to 5db10ae Compare June 4, 2024 21:02
@rdwebdesign rdwebdesign marked this pull request as ready for review June 4, 2024 21:03
@rdwebdesign rdwebdesign requested a review from a team June 5, 2024 01:15
yubiuser
yubiuser reviewed Jun 5, 2024
View reviewed changes

@yubiuser yubiuser left a comment

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.

I really like this change. It looks much nicer with the tabs. But as a user I wouldn't know if I need to save after changing settings in one tab or if I could do changes on multiple tabs and only save once. Any idea how to let the user know?

Comment thread scripts/pi-hole/js/settings-advanced.js
@PromoFaux

PromoFaux commented Jun 5, 2024

Copy link
Copy Markdown
Member

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

yubiuser commented Jun 5, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

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

PromoFaux commented Jun 5, 2024

Copy link
Copy Markdown
Member

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

rdwebdesign commented Jun 5, 2024 •
edited
Loading

Copy link
Copy Markdown
Member Author

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

PromoFaux commented Jun 5, 2024

Copy link
Copy Markdown
Member

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

@rdwebdesign
Do not use tabs when showing only Modified Settings
ab16bb8 
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign
Add button classes to nav-pills elements
8a88c8e 
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign
Format the buttons
8768f6a 
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign
Adapt the style to LCARS theme
20fc994 
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@rdwebdesign rdwebdesign force-pushed the new/settings_all_layout branch from 6303c2d to 20fc994 Compare June 5, 2024 23:32
@rdwebdesign rdwebdesign requested review from a team and PromoFaux June 6, 2024 00:16
@DL6ER

DL6ER commented Jun 6, 2024

Copy link
Copy Markdown
Member

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

@rdwebdesign rdwebdesign merged commit 5b6c8bb into development-v6 Jun 6, 2024
@rdwebdesign rdwebdesign deleted the new/settings_all_layout branch June 6, 2024 22:13
@PromoFaux PromoFaux mentioned this pull request Feb 18, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DL6ER DL6ER DL6ER approved these changes

@yubiuser yubiuser yubiuser approved these changes

@PromoFaux PromoFaux Awaiting requested review from PromoFaux

Assignees

No one assigned

Labels

Pi-hole v6.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@rdwebdesign @PromoFaux @yubiuser @DL6ER