New (additional) LCARS theme inspired by Picard TV show #2709

MichalSvatos · 2023-09-19T20:56:41Z

What does this PR aim to accomplish?

New (additional) LCARS theme. Description and more screenshot available here. Related discussion on the Discourse forum.

Small preview

How does this PR accomplish the above?

New CSS file added and configuration of the themes updated.

By submitting this pull request, I confirm the following:

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.
I have commented my proposed changes within the code and I have tested my changes.
I am willing to help maintain this change if there are issues with it later.
It is compatible with the EUPL 1.2 license
I have squashed any insignificant commits. (git rebase)
I have checked that another pull request for this purpose does not exist.
I have considered, and confirmed that this submission will be valuable to others.
I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
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

style/themes/lcars-picard.css

MichalSvatos · 2023-09-19T21:35:08Z

I'm trying to solve another issue. I included the "Signed-by-off.." message, but it says it's wrong. But as far as I can see those 2 strings are identical 🤔

Expected "Michal Svatoš github@svatos.dev", but got "Michal Svatoš github@svatos.dev"

rdwebdesign · 2023-09-19T21:43:59Z

I don't know what the issue is.

Maybe it's an encoding difference (like this one), or DCO test simply can't handle non-english strings (Svatoš).

MichalSvatos · 2023-09-19T21:48:16Z

It crossed my mind, but I somehow presumed that a global company in 2023 could acount for that. I'll try to search more.

rdwebdesign · 2023-09-19T21:52:32Z

I'm guessing about the encoding, but if I'm right, the name and encoding on your Github account should match the name (and encoding) set on your machine (git config --global user.name).

rdwebdesign · 2023-09-20T00:29:22Z

The "Lint / PHP-CS-Fixer" error is unrelated to this PR.
We will probably fix it on another PR.

MichalSvatos · 2023-09-20T00:32:47Z

Yes, I figure it out 🙂 I went after the prettier fix and the DCO, which was caused by the special characters as you pointed out previously 👍

Sorry, for the mess I had to setup everything for the GH signing and also I didn't realize (which I should have 🤦 ) that the code style will be probably different from mine.

rdwebdesign · 2023-09-20T01:54:32Z

I saw a few visual issues on desktop. Please, confirm if you see the same:

The gray background creating the curve is not correctly aligned (bottom part of the image):
.
I fixed it adding width: calc(var(--sidebar-width) - var(--gap25)); (line 1653).

NOTE: this is happening only on the full width layout.
There is a secondary/internal scrollbar when using the full width layout:
Related to the same footer issue, the style you added to the footer bottom using :after selector is not visible when using boxed layout (it's hidden even when you scroll to bottom):

MichalSvatos · 2023-09-20T02:37:31Z

I see all the issues.

The first two could be solved by adding small padding to the wrapper padding-inline: var(--gap25) and overflow: hidden, but there is probably a reason why overflow-y is set to auto 🤔

The third one ... you're running it in the Docker I presume? Because on the "normal" version there are only 3 items in the footer and I put there the 4th one fill the visual hole and insert self promo at the same time 👼 I'll delete it.

At the same time I noticed another small issue with the footer.

Thank you for the feedback! I'll fix it during tomorrow.

yubiuser · 2023-09-20T11:43:31Z

We will probably fix it on another PR.

#2710

yubiuser · 2023-09-20T11:48:53Z

The notification alert is misaligned (should be attached to the arrows like in the other themes I guess?)

yubiuser · 2023-09-20T11:49:51Z

Some glitches here as well

MichalSvatos · 2023-09-20T13:58:46Z

The notification alert is misaligned (should be attached to the arrows like in the other themes I guess?)

It should look like this:

MichalSvatos · 2023-09-20T14:12:03Z

Some glitches here as well

Is this the same installation? To be honest I don't know how to fix this now from the top of my head 🤔 The theme needs the "Antonio" font. Or at least some "narrow" font. Any ideas?

(I think I will have to get the Docker up and running as soon as possible to be more flexible with the testing.)

yubiuser · 2023-09-20T14:28:21Z

Yes, it's the same installation. I checked out your branch on my docker installation.

rdwebdesign · 2023-09-20T18:23:52Z

I'm not seeing the same issue with the font:

I will investigate.

rdwebdesign · 2023-09-20T19:04:34Z

I don't want to interfere with your design choices, but I think the contrast is too low for the links on Hostname column (network table):

Using something like this will improve the readability:

table[id="network-entries"] td:nth-child(4) a:not(:hover) {
  color: #fff;
}

What do you think?

MichalSvatos · 2023-09-20T19:44:11Z

Scrollbar + footer + rounded corners

The scrollbar was caused by footer (:after pseudoelement was too tall). I added the inline padding which fixed the rounded corners and it looks better. No overflow: hidden needed.

Color in network table

You're right @rdwebdesign - fixed.
(Sidenote - the color palette used in that table is not ideal, but I didn't want to introduce completely new colors just for that. We can think about something later, it's not a deal breaker.)

Number next to the arrow

I fixed the position to acount for bigger numbers, but the missalign in your screenshot is caused by the font again.

Font

Can you check which kind of font is actually applied @yubiuser? I don't know why the "Antonio" font is not used. It's local, so it cannot be some blocker of 3rd party fonts. Maybe some custom settings?

(ℹ️ I rebased the branch to include PHP_CS)

rdwebdesign · 2023-09-20T20:18:26Z

I found by accident (when I resized the browser window) another issue.

On the Dashboard, the legend for Query Types graphic (and probably also Upstream Servers graphic) is overflowing on certain conditions (big number of items and smaller screens, like tablets)

yubiuser · 2023-09-20T20:22:39Z

Can you check which kind of font is actually applied @yubiuser?

Not sure how to check this. Inspection gave me

MichalSvatos · 2023-09-20T21:50:47Z

On the Dashboard, the legend for Query Types graphic (and probably also Upstream Servers graphic) is overflowing on certain conditions (big number of items and smaller screens, like tablets)

Ouch. That's a problem 🤔 I didn't think there could be so much of them. But I knew that the position absolute was risky move.

I quickly tried several things (like calculating height based on viewport width and margins), but we will have to renounce the position absolute. It's not gonna look so great, but it will work.

What about something like this? (quick styling with dev tools)

rdwebdesign · 2023-09-20T21:52:52Z

What about something like this?

What will happen if there are 9 or 10 items?

MichalSvatos · 2023-09-20T21:56:18Z

I'll have to tweak it little bit more, but this version is not based on position: absolute, so it will scale according to the number of items 🙂

MichalSvatos · 2023-09-20T22:31:28Z

2 variants, both with dynamic height. Which one do you think is better @rdwebdesign?

yubiuser · 2023-10-08T17:33:20Z

That's not a DEV version of docker, right? Because in my docker with pihole/pihole-dev it looks fine.

Your're correct, it's the stable v5 image. I'll try it with the dev version

ADD: Using the dev container the footer looks fine

yubiuser · 2023-10-08T17:50:59Z

The filter in Domain Management looks like this (on dev ;-)

Using boxed layout and having a long domain, the audit log will show de-arranged buttons

PromoFaux · 2023-10-08T18:42:23Z

The :dev tag on docker still uses all the master branches of the other components. (It is designed to do docker Dev work against currently released pi-hole)

:nightly has all the Dev branches

MichalSvatos · 2023-10-08T18:49:28Z

Audit log - got it 👍

@PromoFaux So I should use :nightly to test the devel version? Or the :nightly tag also contains v6 changes ...? 🤔 Sorry, for the lame question. I'm still struggling with the docker pi-hole.

PromoFaux · 2023-10-08T18:51:08Z

the nightly tag contains the development branches for v5 (so core development, web devel, FTL development)

:development-v6 contains all the v6 branches (also named development-v6 on all components)

MichalSvatos · 2023-10-08T18:53:09Z

OIC. Thank you for the clarification 👍

I see now the broken footer, btw. There is a different class list-unstyled vs list-inline.

MichalSvatos · 2023-10-12T23:46:16Z

1.4.2

FIX - checkboxes and radios for both lcars classes
FIX - more general selector for footer styling .list-inline --> ul[class*="list-"]
FIX - alert colors, border-radius and close button styling

rdwebdesign · 2023-10-13T00:06:57Z

style/themes/lcars-picard.css

+  ========================================================== */
+/* body[class*="lcars"][class*="lcars"] is a small dirty trick to avoid even more deep "!important hell" */
+
+body[class*="lcars"][class*="lcars"] [class*="icheck-"] > label {


Why did you add the attribute selector ([class*="lcars"]) twice?

Suggested change

body[class*="lcars"][class*="lcars"] [class*="icheck-"] > label {

body[class*="lcars"] [class*="icheck-"] > label {

I made a comment there to clarify - /* body[class*="lcars"][class*="lcars"] is a small dirty trick to avoid even more deep "!important hell" */

The pihole.css contains several lines of code with body:not(.lcars) .... :not adds specificity, so I needed to do the same, but without depending on another class or putting !important everywhere.

I understand the hack, but are you sure it's needed?
You already increased the specificity with body[class*="lcars"].

I will test again another time, but apparently it works without the hack.

Nightly, with lcars or lcars-picard class. If I remove one of the [class*="lcars"] on line 398 - it breaks the checkboxes.

EDIT: Hmm, I see that maybe it doesn't have to be everywhere 🤔 For example the color, or the border. I wanted to be consistent 😄 I'll adjust that.

@MichalSvatos

I finally found time to test your PR again.

When I use the simplified selector, I saw the issue only in "Domains" page (do you see it in other places?).

I noticed there are some things contributing for the issue, but we can fix them instead of adding a new hack or a lot of !important rules.

I pushed a new branch with the changes. ~~I will wait the team approval and them we can simplify the selectors here.~~ (merged)

~~The new code is in development-v6.~~ You can rebase your branch to use the new code.

EDIT:
The new code is in devel.

Yes, it looks like that at the end only the "Domains" page cause an issue.

This PR is based on devel not development-v6, are you sure that I should rebase?

(I'm sorry for such a delay.)

Sorry... it was a distraction when I typed.

New code is in devel.

PromoFaux · 2023-10-22T11:06:00Z

style/themes/lcars-picard.css

+
+/* --- additional visual tweaks */
+.navbar-custom-menu > .navbar-nav > li > .dropdown-menu::after {
+  content: "LCARS theme by @MichalSvatos";


@MichalSvatos - can you remove this please? No issue with the Author attribution in the header of the file.

The Pull Request / commits / release notes will provide credit

Oh, sure! I've already removed one in the footer, I forgot about this one.

yubiuser · 2023-10-25T21:20:44Z

I still think the position of the buttons at the audit log is kind of broken for long domains.

rdwebdesign · 2023-10-25T22:09:04Z

You are right.
I probably didn't have long domains when I tested.

Something like this should fix:

#domain-frequency .table-bordered > tbody > tr > td:last-child,
#ad-frequency .table-bordered > tbody > tr > td:last-child {
  width: 204px;
}

But I'm not sure if this will affect the Long-term Data > Top Lists.

Signed-off-by: Michal Svatos <github@svatos.dev>

MichalSvatos · 2023-10-30T22:34:26Z

I went through few iterations and this one was the good compromise between the functionality and least amount of code added. Other options were unecessaryli complex (eg. turning table rows in a flex containers). And simple white-space: nowrap rendered domains almost unreadable on the smaller screens.

Not perfect solution, but it works. Anyway I open to suggestions.

yubiuser · 2023-11-01T13:16:21Z

Looks good

yubiuser · 2023-11-01T21:56:07Z

@MichalSvatos

I think you could start working to adapt your theme to v6 now.....

MichalSvatos · 2023-11-01T22:59:25Z

I tried to run development-v6 in the docker the same way I did for devel, but it doesn't work 🙁

My `docker_run.sh`

#!/bin/bash

# https://github.com/pi-hole/docker-pi-hole/blob/master/README.md

PIHOLE_BASE="${PIHOLE_BASE:-$(pwd)}"
[[ -d "$PIHOLE_BASE" ]] || mkdir -p "$PIHOLE_BASE" || { echo "Couldn't create storage directory: $PIHOLE_BASE"; exit 1; }

# Note: FTLCONF_LOCAL_IPV4 should be replaced with your external ip.
docker run -d \
  --name pihole-v6 \
  -p 53:53/tcp -p 53:53/udp \
  -p 80:80 \
  -e TZ="America/Chicago" \
  --dns=127.0.0.1 --dns=1.1.1.1 \
  --restart=unless-stopped \
  --hostname pi.hole.v6 \
  -e VIRTUAL_HOST="pi.hole" \
  -e PROXY_LOCATION="pi.hole" \
  -e FTLCONF_LOCAL_IPV4="XXX.XXX.X.XX" \
  -e WEBPASSWORD="123456" \
  -e DNSMASQ_USER=root \
  -e PIHOLE_UID=501 \
  -e PIHOLE_GID=20 \
  pihole/pihole:development-v6

printf 'Starting up pihole container '
for i in $(seq 1 20); do
  if [ "$(docker inspect -f "{{.State.Health.Status}}" pihole)" == "healthy" ] ; then
      printf ' OK'
      echo -e "\n$(docker logs pihole 2> /dev/null | grep 'password:') for your pi-hole: http://${IP}/admin/"
      exit 0
  else
      sleep 3
      printf '.'
  fi

  if [ $i -eq 20 ] ; then
      echo -e "\nTimed out waiting for Pi-hole start, consult your container logs for more info (\`docker logs pihole\`)"
      exit 1
  fi
done;

The error

For security reasons, the password you type will not be visible.

sudo: a terminal is required to read the password; either use the -S option to read from standard input or configure an askpass helper
sudo: a password is required

rdwebdesign · 2023-11-01T23:10:47Z

I never used this script and I never saw this error.
I use only a docker run command or a compose file.

Note:
The new image uses different Environment Variables (and their names are case sensitive).

You can find details in docker-pihole v6 readme:
https://github.com/pi-hole/docker-pi-hole/tree/development-v6

infinitytec · 2023-11-21T23:25:17Z

Nice job!

…f8 (#15274) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [pihole/pihole](https://github.com/pi-hole/docker-pi-hole) | minor | `2023.10.0` -> `2023.11.0` | --- > [!WARNING] > Some dependencies could not be looked up. Check the Dependency Dashboard for more information. --- ### Release Notes <details> <summary>pi-hole/docker-pi-hole (pihole/pihole)</summary> ### [`v2023.11.0`](https://github.com/pi-hole/docker-pi-hole/releases/tag/2023.11.0) [Compare Source](https://github.com/pi-hole/docker-pi-hole/compare/2023.10.0...2023.11.0) Nothing Docker specific, this release pulls in web v5.21 https://github.com/pi-hole/web/releases/tag/v5.21  #### What's Changed - Insert icheck CSS right after bootstrap file. by [@rdwebdesign](https://github.com/rdwebdesign) in [https://github.com/pi-hole/web/pull/2752](https://github.com/pi-hole/web/pull/2752) - New (additional) LCARS theme inspired by Picard TV show by [@MichalSvatos](https://github.com/MichalSvatos) in [https://github.com/pi-hole/web/pull/2709](https://github.com/pi-hole/web/pull/2709) - Add word break for clients column to avoid horizontal scrollbar by [@rdwebdesign](https://github.com/rdwebdesign) in [https://github.com/pi-hole/web/pull/2838](https://github.com/pi-hole/web/pull/2838) #### New Contributors - [@MichalSvatos](https://github.com/MichalSvatos) made their first contribution in [https://github.com/pi-hole/web/pull/2709](https://github.com/pi-hole/web/pull/2709) **Full Changelog**: pi-hole/web@v5.20.2...v5.21 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

rdwebdesign requested changes Sep 19, 2023

View reviewed changes

style/themes/lcars-picard.css Show resolved Hide resolved

MichalSvatos force-pushed the new/additional-lcars-theme branch 3 times, most recently from 7ca5d57 to f310734 Compare September 20, 2023 00:20

MichalSvatos force-pushed the new/additional-lcars-theme branch from f310734 to fb4a888 Compare September 20, 2023 00:38

yubiuser mentioned this pull request Sep 20, 2023

Fix PHP_CS error #2710

Merged

1 task

MichalSvatos force-pushed the new/additional-lcars-theme branch from fb4a888 to d04c422 Compare September 20, 2023 19:34

MichalSvatos force-pushed the new/additional-lcars-theme branch from e8581c7 to 6197573 Compare October 12, 2023 23:45

rdwebdesign requested changes Oct 13, 2023

View reviewed changes

rdwebdesign mentioned this pull request Oct 15, 2023

Insert icheck CSS right after bootstrap file. #2752

Merged

1 task

MichalSvatos force-pushed the new/additional-lcars-theme branch 2 times, most recently from 5d118d8 to a3b203d Compare October 21, 2023 15:17

PromoFaux reviewed Oct 22, 2023

View reviewed changes

MichalSvatos force-pushed the new/additional-lcars-theme branch from a3b203d to e460c85 Compare October 23, 2023 21:23

rdwebdesign previously approved these changes Oct 23, 2023

View reviewed changes

Added new LCARS theme

070ccb5

Signed-off-by: Michal Svatos <github@svatos.dev>

MichalSvatos dismissed rdwebdesign’s stale review via 070ccb5 October 30, 2023 22:29

MichalSvatos force-pushed the new/additional-lcars-theme branch from e460c85 to 070ccb5 Compare October 30, 2023 22:29

yubiuser approved these changes Nov 1, 2023

View reviewed changes

rdwebdesign merged commit e38522b into pi-hole:devel Nov 1, 2023
10 checks passed

PromoFaux mentioned this pull request Nov 21, 2023

v5.21 #2860

Merged

	body[class="lcars"][class="lcars"] [class*="icheck-"] > label {
	body[class="lcars"] [class="icheck-"] > label {

New (additional) LCARS theme inspired by Picard TV show #2709

New (additional) LCARS theme inspired by Picard TV show #2709

Conversation

MichalSvatos commented Sep 19, 2023

What does this PR aim to accomplish?

How does this PR accomplish the above?

MichalSvatos commented Sep 19, 2023

rdwebdesign commented Sep 19, 2023

MichalSvatos commented Sep 19, 2023

rdwebdesign commented Sep 19, 2023

rdwebdesign commented Sep 20, 2023

MichalSvatos commented Sep 20, 2023

rdwebdesign commented Sep 20, 2023 • edited Loading

MichalSvatos commented Sep 20, 2023

yubiuser commented Sep 20, 2023

yubiuser commented Sep 20, 2023

yubiuser commented Sep 20, 2023 • edited Loading

MichalSvatos commented Sep 20, 2023

MichalSvatos commented Sep 20, 2023 • edited Loading

yubiuser commented Sep 20, 2023

rdwebdesign commented Sep 20, 2023

rdwebdesign commented Sep 20, 2023

MichalSvatos commented Sep 20, 2023 • edited Loading

Scrollbar + footer + rounded corners

Color in network table

Number next to the arrow

Font

rdwebdesign commented Sep 20, 2023

yubiuser commented Sep 20, 2023

MichalSvatos commented Sep 20, 2023

rdwebdesign commented Sep 20, 2023

MichalSvatos commented Sep 20, 2023

MichalSvatos commented Sep 20, 2023

yubiuser commented Oct 8, 2023 • edited Loading

yubiuser commented Oct 8, 2023

PromoFaux commented Oct 8, 2023

MichalSvatos commented Oct 8, 2023

PromoFaux commented Oct 8, 2023

MichalSvatos commented Oct 8, 2023

MichalSvatos commented Oct 12, 2023

rdwebdesign Oct 13, 2023

Choose a reason for hiding this comment

MichalSvatos Oct 13, 2023

Choose a reason for hiding this comment

rdwebdesign Oct 13, 2023

Choose a reason for hiding this comment

MichalSvatos Oct 13, 2023 • edited Loading

Choose a reason for hiding this comment

rdwebdesign Oct 15, 2023 • edited Loading

Choose a reason for hiding this comment

MichalSvatos Oct 19, 2023 • edited Loading

Choose a reason for hiding this comment

rdwebdesign Oct 19, 2023

Choose a reason for hiding this comment

PromoFaux Oct 22, 2023

Choose a reason for hiding this comment

MichalSvatos Oct 22, 2023

Choose a reason for hiding this comment

yubiuser commented Oct 25, 2023

rdwebdesign commented Oct 25, 2023

MichalSvatos commented Oct 30, 2023 • edited Loading

yubiuser commented Nov 1, 2023

yubiuser commented Nov 1, 2023

MichalSvatos commented Nov 1, 2023

rdwebdesign commented Nov 1, 2023

infinitytec commented Nov 21, 2023

rdwebdesign commented Sep 20, 2023 •

edited

Loading

yubiuser commented Sep 20, 2023 •

edited

Loading

MichalSvatos commented Sep 20, 2023 •

edited

Loading

MichalSvatos commented Sep 20, 2023 •

edited

Loading

yubiuser commented Oct 8, 2023 •

edited

Loading

MichalSvatos Oct 13, 2023 •

edited

Loading

rdwebdesign Oct 15, 2023 •

edited

Loading

MichalSvatos Oct 19, 2023 •

edited

Loading

MichalSvatos commented Oct 30, 2023 •

edited

Loading