Skip to content

switch to using the retro logo in place of the green logo #454

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

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

mwoolweaver
Copy link
Contributor

@mwoolweaver mwoolweaver commented Mar 31, 2025

What does this PR aim to accomplish?:

switch to using the retro logo in place of the green logo as they appear, from my limited testing, to be the same size logo.

  • green logo was used in the following places:

    · as the logo for regular

    · during the start up of regular and slim

moves moveXOffset from StartupRoutine to PrintLogo for consistency.

removes version check at the begining of PrintLogo() as version info is not available yet.

for mini size replace *_status variable in PrintLogo() with description provided with big logo,
for smaller than mini size, removes *_status variable as they aren't available yet.


before with regular
Screenshot From 2025-03-31 17-35-55

after with regular
Screenshot From 2025-03-31 17-32-12


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

@mwoolweaver mwoolweaver force-pushed the no-big-green-padd-logo branch from 2484a64 to 095c235 Compare April 4, 2025 00:59
@yubiuser yubiuser mentioned this pull request Apr 4, 2025
@mwoolweaver mwoolweaver force-pushed the no-big-green-padd-logo branch from 095c235 to fa17934 Compare April 5, 2025 22:44
Copy link
Member

@yubiuser yubiuser left a comment

Choose a reason for hiding this comment

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

I think you can remove even more code.

In PrintLogo()only pico up to mini will ever be passed. The rest can be removed.

@mwoolweaver
Copy link
Contributor Author

mwoolweaver commented Apr 8, 2025

so remove

PADD/padd.sh

Lines 1083 to 1087 in 2eb1057

elif [ "$1" = "tiny" ]; then
printf "%s${clear_line}\n" "${padd_text}${dim_text}tiny${reset_text} ${version_info}${reset_text}"
printf "%s${clear_line}\n" " PADD ${padd_version_heatmap}${padd_version}${reset_text} ${tiny_status}${reset_text}"
elif [ "$1" = "slim" ]; then
printf "%s${clear_line}\n${clear_line}\n" "${padd_text}${dim_text}slim${reset_text} ${full_status}"

or all the way down to

PADD/padd.sh

Line 1092 in 2eb1057

# normal or not defined

@yubiuser
Copy link
Member

yubiuser commented Apr 8, 2025

All down. Just search for the occurrence of printLogo and the arguments that get passed.

@mwoolweaver mwoolweaver force-pushed the no-big-green-padd-logo branch from e0d72db to 7e7e2c0 Compare April 8, 2025 19:59
@mwoolweaver
Copy link
Contributor Author

i checked StartupRoutine() and saw what you mentioned about passed args and removed everything after mini.

@mwoolweaver mwoolweaver force-pushed the no-big-green-padd-logo branch 2 times, most recently from bbd11bf to 0a607e3 Compare April 8, 2025 20:17
@mwoolweaver mwoolweaver force-pushed the no-big-green-padd-logo branch 6 times, most recently from 5f44d03 to ed0876e Compare April 13, 2025 18:58
@mwoolweaver mwoolweaver requested a review from yubiuser April 13, 2025 18:59
@mwoolweaver
Copy link
Contributor Author

mwoolweaver commented Apr 13, 2025

Can the Docker check be removed from the beginning of PrintLogo()? When PrintLogo() is called the version info hasn't been fetched.

PADD/padd.sh

Lines 1068 to 1072 in 2eb1057

if [ ! "${DOCKER_VERSION}" = "null" ]; then
version_info="Docker ${docker_version_heatmap}${DOCKER_VERSION}${reset_text}"
else
version_info="Pi-hole® ${core_version_heatmap}${CORE_VERSION}${reset_text}, Web ${web_version_heatmap}${WEB_VERSION}${reset_text}, FTL ${ftl_version_heatmap}${FTL_VERSION}${reset_text}"
fi

The exact same check can be seen here as well in PrintDashboard()

PADD/padd.sh

Lines 1101 to 1105 in 2eb1057

if [ ! "${DOCKER_VERSION}" = "null" ]; then
version_info="Docker ${docker_version_heatmap}${DOCKER_VERSION}${reset_text}"
else
version_info="Pi-hole® ${core_version_heatmap}${CORE_VERSION}${reset_text}, Web ${web_version_heatmap}${WEB_VERSION}${reset_text}, FTL ${ftl_version_heatmap}${FTL_VERSION}${reset_text}"
fi

@mwoolweaver mwoolweaver force-pushed the no-big-green-padd-logo branch 5 times, most recently from cc495ea to 3cb4bb3 Compare April 14, 2025 02:35
 - green logo was used in the following places:

   · as the logo for regular

   · during the start up of regular and slim

moves `moveXOffset` from `StartupRoutine` to `PrintLogo` for consistency.

removes version check at the begining of `PrintLogo()` as version info is not available yet.

for mini size replace `*_status` variable in `PrintLogo()` with description provided with big logo,
for smaller than mini size, removes `*_status` variable as they aren't available yet.

Signed-off-by: Michael Woolweaver <[email protected]>
@mwoolweaver mwoolweaver force-pushed the no-big-green-padd-logo branch from 3cb4bb3 to 37a9ab8 Compare April 14, 2025 03:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants