Skip to content

Conversation

logonoff
Copy link
Member

@logonoff logonoff commented Jun 20, 2025

Description

Modifies app.branding.fullLogo and app.branding.iconLogo to also accept an object that contains light and dark.

RHDH now has this behaviour for the sidebar logo:

  • if app.branding.fullLogo is a string, it will be shown in both light and dark theme.
  • if app.branding.fullLogo is an object, the image associated with dark key will be shown in application dark theme, and vice versa

RHDH now has this behaviour for the favicon:

  • if app.branding.iconLogo is a string, it will be shown in both light and dark theme.
  • if app.branding.iconLogo is an object, the image associated with dark key will be shown when the system is in dark theme (that is we use the prefers-color-scheme media query), and vice versa

Moreover, the default RHDH app.branding.fullLogo and app.branding.iconLogo has been removed in favour of updating the fallback LogoFull and LogoIcon from Janus branding to RHDH.

This behaviour is similar to other Red Hat products, such as OCP console, as described in openshift/enhancements#1753

demo:

Screencast.From.2025-06-20.10-41-47.mp4

(note the kiwi, banana, and non-official RHDH logo do not reflect the app-config changes I made--the app-configs I changed align with brand)

Which issue(s) does this PR fix

https://issues.redhat.com/browse/RHIDP-7734, https://issues.redhat.com/browse/RHIDP-3227, https://issues.redhat.com/browse/RHIDP-7593, https://issues.redhat.com/browse/RHDHPAI-907

PR acceptance criteria

Please make sure that the following steps are complete:

  • GitHub Actions are completed and successful
  • Unit Tests are updated and passing
  • E2E Tests are updated and passing
  • Documentation is updated if necessary (requirement for new features)
  • Add a screenshot if the change is UX/UI related

How to test changes / Special notes to the reviewer

  • also needs test on RHDH local / cluster

@openshift-ci openshift-ci bot requested review from albarbaro and dzemanov June 20, 2025 14:48
@logonoff logonoff force-pushed the light-rhdh branch 2 times, most recently from c5d7f6d to 415797d Compare June 20, 2025 14:56
Copy link
Contributor

@logonoff logonoff changed the title fix(SidebarLogo): support both light and dark variants (wip) fix(SidebarLogo): support both light and dark variants Jun 20, 2025
Copy link
Contributor

@logonoff logonoff changed the title fix(SidebarLogo): support both light and dark variants feat: support both light and dark logo variants Jun 20, 2025
Copy link
Contributor

Copy link
Contributor

Copy link
Contributor

Copy link
Contributor

Copy link
Contributor

@invincibleJai
Copy link
Member

/cc @its-mitesh-kumar
/cc @ciiay

@openshift-ci openshift-ci bot requested review from ciiay and its-mitesh-kumar June 23, 2025 13:37
Copy link
Contributor

Copy link
Contributor

@logonoff logonoff force-pushed the light-rhdh branch 5 times, most recently from 475f65c to 3c66089 Compare June 23, 2025 17:56
Copy link
Contributor

@its-mitesh-kumar
Copy link
Member

its-mitesh-kumar commented Jun 25, 2025

@logonoff , I am adding custom fullLogo, but I am seeing default RHDH Logo. Here is the screenshot
Screenshot 2025-06-25 at 1 02 53 PM
Configuration is below

app:
  sidebar:
    search: false
    settings: false
    logo: false
  branding:
    fullLogo: 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAANoAAABQCAMAAABI6JtXAAAB11BMVEUAAAC/ICDEIh7FIh/FIh/EIh/FIx/HIx77uwT7vAT7vAP7vAT7vAT8vAP/vwDHIyDGIh6/ICD/vwD7vQT7vAT8vAX3vwD6vAPFIh/FIh/GIh76vAXrRDXfQDDrQjX/twjCIx3qQzXqQzXnSDD8vgNAQEBASEhFRUVESEjCIB3qQzXoQjftQzXrQzT6vQNDR0VFR0ZER0ZER0ZERkZER0VGRkZCR0RERkZERkVER0ZER0ZER0ZESERFR0ZISEhDR0dDR0ZDR0ZER0ZDSEZERkVER0ZESEZASEBCSEVDR0VDR0WCUoq0LjriuQ51rThERkVDRkZChfJSedmcQWLJtxhBqU4yp1JFRUVARUVChfSEU4qLsTA0qFNbcsxmrT9DRkRGRkZFR0ZER0dDR0dESEhDR0VAQEBFR0dER0ZASEhER0VER0VDR0VERkVDR0VER0brQzTqQzRHR0dESERDR0bpRDToRDRER0dCR0TqQDXpQjXnRDTnQDjfQEBERkZERkZISEhFRUVFSEVDRkNFRUVDRkRCSEVFR0VFR0VFSEhFSEhARUVDRkZER0dFR0ZER0ZDR0VFSEVDg/I0p1RAgO9BhfQwp1BAhPE0p1RAhfJBhPQ0p1IzqVO/3uLxAAAAnXRSTlMAIJDf/++/X0C/7//PnyBfjxAQj+9fIN/P73BvjxCPIFD/3yBPECAwQGDvcG/vYJ/f/89/v19wgO8Bj/5AASCer9/vXwGvgCBgAZBg//9gz1Bg/////2BgMP///////6BQ3nCfQXARn84hAb5v7o/ur79PQd6AcE9xMN9AIBABoCExYFBhAWGeb2BhMa9v3f2eX1BAEO8gcIBgz89fZda5bQAABHNJREFUeAHs0YMBAwEQBMB742LjrTj9dxfb5k4FC4KHAgAAAAAAABBESVZUjS6kGyan0hn6AllFnsnl6QKFIs+Uyl/wmbwkCXRWpcpLOn26mrxSb9AZTZNXjG85bc6y6QTb4U0ufTZvq5ofhHRUGPi8KaJLxEmrndA7dLar+d0eHdEf+NvVhmNm7EK5bSCKwvB9GckatK6tVEpWYeYyMzOGGcrcN653naNIE8FK2cA/1Bpm/I2OoC1mOXWXVY26Q8Zqul4Hyc74vm/p0lphlAhjlO+VolnNgGO5HWSkzi5mFiERdbvMwi5BwygRxliW5riMgOsxQmNZbyVacpQYY1lak9sFXr3P62fVwKAB2pDawnA1WnKUGGMZGmTByCip7LG2rkaHbnxCDnKwmDY5lUbDKDHGNNr0TKFsxKL9RuQLZKKRWe8sFdPOnb+QQsMoMcYU2sVLlymzKxLWf5UShQFkRiqmXbt2PY2GUWKMoKEbN29m03pcKbMpGYF6fLRrt6ZSaGqUGCOKxnj7Zh7tDreqDjFIuxaNEgSMEmNE0Rhv5tF6WP+0smqOY6X/DdkOXqxAwyhByywaYy7tipwj5dbj+/5dop4JdSXfe7hwPJbd6yAkXwtY1mj2RH7f9+/r0jDKYhrGmEuTv/AB5fZQ3XUf8V6iSfT4CaOnVuzJA7mSI3sm+bo0jLKYhjHm0rhVWEx74HOUuP/c4/1ekAqvwV+BhlEW0zDGPFqNW1ExLZBT9F9O9KsHlS75ysRLv08pQhyz9md8r+2vQMMoi2gYYy7NkesppMlehbiZq163f7mkvog+9MLef/WNVZ6GUb7Nl73DGEHLvIp4FMtNdO89frUIsTvIVB/kWwox1s9zhDMswGErR0M0nydboJuVaJxI3Aetl2IYnrNiz79ikVShTWgJ36hIo+WVLNjKKunTGtk09vdoooYPPMZBiw6iuEIo/qGnh6HR2tuMMa4TaBqXkYGMQQYx2htCnVinamMzRnOa6sbWGO7YgqgiTTafPkbSpVncyqbU8Ksl7UWSZqfQtjyOcg3QkqPEGPVpJH/Elcy7uQi1adsBxzJBS44SY9SnTeA6knpAhaVL21KyYGJnd3dnot8MLTFKjFGfFmY+jnzEpUCH9umOurrgwvnZEA2jxBi1aZgdNzKmKgZ1aY9xg8CHDNEwSoxRn4bD1kUH8kHQoj1MvEoPTdAwSoyxNI3Ug2Az9T9MRqgUjZCapwEaRokxlqVZ6qSv9yRe61LPHKRN28Y1B0fcCA2jXKdqNLL78R/GyHGVzCYqda59iWRGaag8DTYO6l+dmu18mwgYMk0anprv3bXI+u7x6aGRPcsHempTCZp62I8SI6eGpv5JkigYsagUjX7s28TrzlNEI3tsNh2mSyP7pxAK9uoXdQohfisa/tB9T4g/x0+Dbteve331iZ1Rqpb9sv7i7+t/FhVnnnZq+9/+fNAAAMMwEDR/7e4FthysFSl/DK74tarYWrdrQ8FNt7YU3j5O7T4BAAAAAIAUPnsb6R/sjg4HAAAAAElFTkSuQmCC'

@christoph-jerolimov
Copy link
Member

Yep, I just wanted report the same. With light and dark it works fine. But when I use a simple string (old format) it doesn't work as expected.

I saw also two other issues, but I expect this should be fixed in the global-header:

image

The outer div of the company logo has a width of 224px. But the padding on the left has also 24px. I guess the design team inlcudes this 24px in their "it should be 224px wide". So I expect we can go with 200px width on the outer div.

And then I see there the issue that the icon is neigher left or center aligned. The inner logo has a max-width of 150px. Couldn't we just use the full width? Which means 176px at the moment, right?

image

For this PR it's required that the old configuration is supported. I can create a bug for the sizing/alignment issue if needed.

Copy link
Contributor

Copy link
Contributor

@logonoff
Copy link
Member Author

logonoff commented Jun 25, 2025

Yep, I just wanted report the same. With light and dark it works fine. But when I use a simple string (old format) it doesn't work as expected.

The outer div of the company logo has a width of 224px. But the padding on the left has also 24px. I guess the design team inlcudes this 24px in their "it should be 224px wide". So I expect we can go with 200px width on the outer div.

And then I see there the issue that the icon is neigher left or center aligned. The inner logo has a max-width of 150px. Couldn't we just use the full width? Which means 176px at the moment, right?

fixed (using @its-mitesh-kumar's app-config):

image

logonoff added 3 commits June 25, 2025 15:24
fixes an "oepsie woepsie," if you will
includes all the changes in RHDH PR 2893, but a newer version to prevent a runtime error
…nd application launcher

includes changes from PR 3039
Copy link
Contributor

Copy link
Member

@christoph-jerolimov christoph-jerolimov left a comment

Choose a reason for hiding this comment

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

Works great!

Tested it with the latest global-header plugin and changes from the default yaml on a local rhdh.

/lgtm
/approve

Copy link

openshift-ci bot commented Jun 26, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: christoph-jerolimov, ciiay

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [christoph-jerolimov]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@christoph-jerolimov christoph-jerolimov merged commit 84665bd into redhat-developer:main Jun 26, 2025
13 of 14 checks passed
Copy link

openshift-ci bot commented Jun 26, 2025

@logonoff: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-tests 8e3a01e link unknown /test e2e-tests

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@logonoff logonoff deleted the light-rhdh branch June 26, 2025 11:20
chadcrum pushed a commit to chadcrum/rhdh that referenced this pull request Aug 3, 2025
* fix(SidebarLogo): support both light and dark variants

* fix(favicon): support light and dark variants

* feat: replace janus fallback with RHDH, remove duplicated RHDH logo from app config

also part of https://issues.redhat.com/browse/RHIDP-3227

* fix: update the theme again, do not target classnames

fixes an "oepsie woepsie," if you will

* feat(global-header): add CompanyLogo to global-header

includes all the changes in RHDH PR 2893, but a newer version to prevent a runtime error

* feat(global-header): update default header, enable starred dropdown and application launcher

includes changes from PR 3039
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants