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

Dynamic palette preview swatches #16291

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

cconard96
Copy link
Contributor

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -

With all core palettes now being loaded at all times with their application depending on the data-glpi-theme attribute on the root element, it is easy to automatically generate a palette preview "image" for each palette rather than relying on a png file existing for the palette and pulling them all over the network.

GLPI WILL still fall back to trying to load a PNG in a "previews" subfolder where the theme exists. However, if a palette CSS defines 4 palette color properties --glpi-palette-color-n, we will simply use those colors.

After migrating all of the core palettes, it seemed like the loading performance of the palette dropdown improved rather than decreased.

This PR also enables automatic loading of custom palettes. When users create custom palettes they need to follow these rules:

  1. The palette stylesheet needs to have a scss extension
  2. The only top-level selector in the stylesheet should be :root[data-glpi-theme="palette_name"] { where palette_name matches the scss file name without the extension.
    Inside the root selector they can declare their css property overrides and, if they choose, declare the --glpi-palette-color-n properties if they want a preview but don't want to create a png image for it.

Copy link
Contributor

@orthagh orthagh left a comment

Choose a reason for hiding this comment

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

1st thing I though about the pr changes is how var are replicated between, for example, glpi-palette-color-1 and glpi-mainmenu-bg.
For this example, it should be the same value on all palettes and maybe we can set obvious var directly "inheriting" from the n values

@cconard96
Copy link
Contributor Author

1st thing I though about the pr changes is how var are replicated between, for example, glpi-palette-color-1 and glpi-mainmenu-bg. For this example, it should be the same value on all palettes and maybe we can set obvious var directly "inheriting" from the n values

I couldn't think of a good way to use the existing CSS properties tor the preview colors. Top-left widely matches the main menu background while the bottom-right typically matches the primary color. Beyond that, the other current colors don't relate to anything in most cases. The only other colors widely used within the palettes are the body background color and body color which would be white and black in almost every palette.

@cedric-anne
Copy link
Member

@cconard96 @orthagh
Is this PR still relevant ?

@cconard96
Copy link
Contributor Author

I think this could still be useful, but there are some complexities. The variables used for the preview have to have simple values to avoid trying to resolve values manually across multiple stylesheets.

At the very least, I think the automatic loading of custom palettes part of this PR could be moved to its own PR so that both core and custom palettes are always loaded and easily switchable just by changing the attribute on the document root.

@cconard96 cconard96 force-pushed the enhance/drop_theme_preview_pics branch from 2e96c2f to 2605b7f Compare October 19, 2024 17:04
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.

3 participants