Skip to content
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

Custom styling for other distributions #531

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

garrett
Copy link
Contributor

@garrett garrett commented Nov 25, 2024

Building on the Fedora customizations, this extends the concept and simplifies it for others to do simple theming without having to know CSS. (The CSS is abstracted away and we present CSS custom properties, so they can just change a few variables for a huge effect.)

@garrett garrett requested a review from KKoukiou November 25, 2024 16:39
@garrett
Copy link
Contributor Author

garrett commented Nov 25, 2024

I had this based on top of Fedora, so I need to rebase it to main.

@garrett
Copy link
Contributor Author

garrett commented Nov 25, 2024

(I also had the thought that I could rebase the Fedora theme CSS on top of the changes introduced in this as well, so it could inherit from the custom theming import and then stack a few more changes on top, so there'd be less to maintain everywhere. But this can come later and shouldn't affect things much.)

src/branding/fedora.scss Outdated Show resolved Hide resolved
***/

/* Override the top header */
.pf-v5-c-page__main-group > .pf-v5-c-page__main-section:first-child {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should add a class to the main heading and use it here, instead of this.

}
}

#toggle-kebab {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should probably use a class for this also.

display: none;
}

h1 {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should probably also turn this into a class instead.

Comment on lines 3 to 7
// FIXME: Make this automatically distro-specific
@import "../branding/fedora";
/* @import "../branding/fedora"; */
/* @import "../branding/centos"; */
@import "../branding/bazzite";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's the part where it should be automatic. Manually importing is fine for testing, of course. We want to make sure it's not something other than Fedora (either automatic or even manual for now) when merging, however.

Comment on lines +16 to +23
/*
Size of the logo
*/
--logo-height: 3rem;
--logo-width: 3rem;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I should probably have this have a default, and only probably specify width (with an override for height).

Copy link
Contributor

@KKoukiou KKoukiou left a comment

Choose a reason for hiding this comment

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

The changes look fine, but I would not like to merge this before these distros are consumers of Web UI.

Then, encode using URI encoding (not base64) and use as a
background. There's a useful tool at https://svgencode.com/
*/
--logo: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 128 128'%3E%3Cg stroke-width='.3' paint-order='fill markers stroke' transform='matrix(3.1840795,0,0,3.1840795,-1171.916,804.24614)'%3E%3Ccircle cx='435.1' cy='-124.1' r='20.1' fill='%23a14f8c' transform='rotate(-15)'/%3E%3Cpath fill='%23fff' d='m388-248-4 5h-7v6l-4 4 4 5v6h7l4 5 5-5h6v-6l4-5-4-4-6-6zm1 5v5l-1 1-1-1v-5zm-8 2 4 3v2h-2l-3-3zm-4 7h5l2 1-2 2h-5zm17 0h5v3h-5l-1-2zm-11 5h2v2l-3 3-2-2zm8 0h2l4 3-2 2-4-3zm-3 1 1 1v5h-2v-5z'/%3E%3Cpath fill='%23efa724' d='m389-236-1 2-1-2v2h-2l1 1-1 2h2v2l1-2 1 2v-2h2l-1-2 1-1h-2zm4-7 2 2-4 3v2h2l4-3 2 2v-6z'/%3E%3C/g%3E%3C/svg%3E");
Copy link
Contributor

Choose a reason for hiding this comment

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

@garrett I don't think we can include logos like this into out packaging.

See https://github.com/cockpit-project/cockpit/blob/main/doc/branding.md#branding

@garrett
Copy link
Contributor Author

garrett commented Jan 8, 2025

Note: This is a draft PR meant as a proof of concept and has logos and branding based on various downstream distributions, to make sure we can build something they can easily use.

This will need to be merged in eventually in a different state, where it's just the framework and an example.

It also would need some code to hook up detection of a file (and optionally a logo graphic, if it's not inlined in the CSS) to be able to work.

It would also be ideal to rebase the Fedora implementation on top of this framework so it's using the "API" of CSS variables instead of overriding things itself.

I can work on the rebasing of the Fedora theme on top of the "framework", but I need someone else to handle the detection of files for inclusion as well as the RPM packaging.

We also need someone to reach out to the distributions with the examples and have them build appropriate RPMs. Perhaps we can have them use existing RPMs, but with different files. CentOS and RHEL do this as was originally intended (having an alternate RPM) whereas Bazzite (and probably the rest of uBlue distributions) hack the fedora-logos RPM and also overwrite various Fedora files as well, instead of having a separate package.


In other words, here's the TODO:

  1. Refine PR: Remove branding, leave framework & example
  2. Rebase Fedora implementation on CSS API
  3. Add file and logo detection
  4. Handle RPM packaging
  5. Collaborate with distributions on RPMs (possibly reusing branding RPMs where they exist somehow)

I can do # 1 and 2 in this PR, but other people need to do the rest.

@garrett
Copy link
Contributor Author

garrett commented Jan 8, 2025

Additionally, while not fully related, we're using a hack to make the Fedora logo work for us right now. It's hardcoded and Fedora specific:

https://github.com/rhinstaller/anaconda-webui/blob/main/src/branding/fedora.scss#L78-L83

The proper fix would be to have a light version to use on dark backgrounds, then use that (instead of filtering the dark version for light backgrounds).

Bug opened @ https://pagure.io/fedora-logos/issue/26

Meanwhile, things look correct in main and should be shippable for Fedora, even though it's a double-hack for now. (We're hardcoding Fedora for the time being and we're manipulating the Fedora sprite with a filter.)

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.

2 participants