Skip to content

Conversation

@alexmathieu22
Copy link
Contributor

Summary

This PR is a rebase of this PR.

This PR adds accent colors / logo / warning message for each cluster by adding an "Appearance" section in the settings of the clusters. The color change was kept to a minimum as to not disrupt the design, but let us modify it further when a more elaborate design is approved by board. The warning message is optional and it's color is the accent color decided by user. Finally, the icon is currently an icon from the iconify library. This choice was made to kept the change as small as possible, and to use an already used library.

Related Issue

Fixes #15

Changes

  • Added accent color by cluster
  • Added warning message by cluster
  • Added icon by cluster

Steps to Test

  1. Navigate to cluster settings
  2. Modify appearance as wanted
  3. Click on apply
  4. Confirm
  5. Verify change is persisted for cluster only and looks as required

Screenshots (if applicable)

  • See thread of rebased PR

Notes for the Reviewer

  • This touches the i18n layer, so please check language consistency.

@k8s-ci-robot k8s-ci-robot requested review from illume and sniok January 15, 2026 03:18
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 15, 2026
@joaquimrocha
Copy link
Contributor

I only noticed now that you have backend code related to this appearance PR. I do not think that's where the appearance should be stored.
We have many UI related settings (like the number of rows to show by default in tables, and even the allowed namespaces) set in the local storage. We are planning on adding some ways to then distribute such settings more or less automatically via a ConfigMap or similar.
So please keep this PR frontend only (use a similar way to store the cluster appearance settings like we are doing for the ones I mentioned above), in order to make it more in line with the other settings and the code safer as well.

@alexmathieu22
Copy link
Contributor Author

alexmathieu22 commented Jan 15, 2026

Does that mean I should also remove the warning banner? The whole point of this MR was to have a unified setup for all devs. Putting this in localstorage would remove the main idea of adding a warning.

I can do it via localstorage, but do you have a link to the issue / thread regarding:

We are planning on adding some ways to then distribute such settings more or less automatically via a ConfigMap or similar.

This is something that I wouldn't mind working on and is quite important for my team to continue using headlamp going forward.

@alexmathieu22 alexmathieu22 force-pushed the feature/add-appearance-configuration-per-cluster-rebased branch 3 times, most recently from f4da90c to 1ccb8eb Compare January 17, 2026 22:58
@alexmathieu22
Copy link
Contributor Author

alexmathieu22 commented Jan 17, 2026

Hi @joaquimrocha @skoeva ! Following your suggestions, I've modified the PR to only include frontend changes, the setting is now applied in the localstorage.

I've also removed the warning banner to keep changes to the minimum.

Here is a demo of the final result:

Screen.Recording.2026-01-17.at.3.58.24.PM.mov

Please let me know how that looks.

Regarding being able to share those settings across users, is there something I could look into to follow progress / be able to help in any way (edit: saw the following issue: #3979)? This is something that would be very useful for me and my team to use this - it is a big UX enabler for devs having access to a shared headlamp instance to be able to easily see they are working on a prod cluster without having to set those settings themselves :)

@alexmathieu22 alexmathieu22 force-pushed the feature/add-appearance-configuration-per-cluster-rebased branch from 1ccb8eb to e7b371c Compare January 17, 2026 23:35
@alexmathieu22
Copy link
Contributor Author

alexmathieu22 commented Jan 21, 2026

@illume @skoeva @joaquimrocha @sniok any updates regarding this PR?

@joaquimrocha
Copy link
Contributor

Hi @alexmathieu22 . Thanks for the PR clean up and updates. I think it looks pretty good in your video. I am a bit concerned how a larger group of clusters will look in the side bar, but I think that the rest should be mergeable. I want to take a closer look and unfortunately didn't have time for it at all this week. Maybe next week I will able to, as I really want to have this.
@sniok , WDYT?

@sniok
Copy link
Contributor

sniok commented Jan 28, 2026

image

This button in the sidebar serves so many purposes: link to the cluster overview page, indicator for selected clusters, button that expands submenu. And now with icons it just looks way too "busy". Maybe as a interim solution (before we rework how cluster selector works and moved to the sidebar) we can make it so clusters don't have icons by default, so without manually assigning icons it will look more or less like it used to, what do you think?

@joaquimrocha
Copy link
Contributor

joaquimrocha commented Jan 28, 2026

Maybe as a interim solution (before we rework how cluster selector works and moved to the sidebar) we can make it so clusters don't have icons by default, so without manually assigning icons it will look more or less like it used to, what do you think?

I assume you are suggesting we keep the icon for the cluster if it's just 1 cluster being shown), ideally replacing the sidebar icon we now have; and if more than one, we don't show the cluster icon at all (keep it as it was).
If that is the case, I agree.

@sniok
Copy link
Contributor

sniok commented Jan 30, 2026

I assume you are suggesting we keep the icon for the cluster if it's just 1 cluster being shown), ideally replacing the sidebar icon we now have; and if more than one, we don't show the cluster icon at all (keep it as it was). If that is the case, I agree.

I'm suggesting that clusters by default don't have an icon. Currently it adds default icons to every cluster.

image

@illume
Copy link
Contributor

illume commented Jan 31, 2026

I'm suggesting that clusters by default don't have an icon.

Yeah, I think this will look better by default (to have no icons).

It could be a default minikube, kind, k3d (and other testing clusters) icons could be good. But even then some people have like 10 test clusters... so it would have the same problem of lots of un-unique icons.

@illume illume added this to the v0.40.0 milestone Jan 31, 2026
@illume illume added kind/feature Categorizes issue or PR as related to a new feature. frontend Issues related to the frontend labels Jan 31, 2026
Copy link
Contributor

@illume illume left a comment

Choose a reason for hiding this comment

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

🎉 thanks!

(I'd like to get this in for 0.40.0 release on Thursday, code freeze on Tuesday. Might I suggest we take further design discussion to an issue? It seems to me a good improvement as it is, and we can iterate on this if needed in the next releases. Sound like a plan?)

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 31, 2026
@alexmathieu22 alexmathieu22 force-pushed the feature/add-appearance-configuration-per-cluster-rebased branch from e7b371c to 97b798d Compare January 31, 2026 20:47
@alexmathieu22
Copy link
Contributor Author

Good idea to have no default icon, just modified it:
Here's the final look:

with no icon set:
image
image
image
image

with an icon set for a cluster:
image
image
image
image

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 2, 2026
@joaquimrocha
Copy link
Contributor

@illume I have approved this changes. I think there are a couple of things we could improve but it's also in a good state already and I love having icons for clusters.
@alexmathieu22 , if you can solve the current conflict, we'll then merge your PR. Thanks.

Copy link
Contributor

@sniok sniok left a comment

Choose a reason for hiding this comment

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

lgtm!

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alexmathieu22, illume, joaquimrocha, sniok

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

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [illume,joaquimrocha,sniok]

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

@alexmathieu22 alexmathieu22 force-pushed the feature/add-appearance-configuration-per-cluster-rebased branch 4 times, most recently from ae8b927 to 971270f Compare February 3, 2026 01:19
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 3, 2026
@alexmathieu22 alexmathieu22 force-pushed the feature/add-appearance-configuration-per-cluster-rebased branch from 9fd4ed0 to 163516f Compare February 3, 2026 01:51
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 3, 2026
@alexmathieu22 alexmathieu22 force-pushed the feature/add-appearance-configuration-per-cluster-rebased branch from 163516f to cc2596d Compare February 3, 2026 01:53
@joaquimrocha joaquimrocha force-pushed the feature/add-appearance-configuration-per-cluster-rebased branch 2 times, most recently from cc2596d to 5d68c0e Compare February 3, 2026 14:39
@joaquimrocha joaquimrocha force-pushed the feature/add-appearance-configuration-per-cluster-rebased branch from 5d68c0e to 67dfb99 Compare February 3, 2026 15:54
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 3, 2026
@joaquimrocha
Copy link
Contributor

@alexmathieu22 I removed your merge branch hoping I am helping you land this PR.
I am having trouble with headlamp in my own machine because it keeps generating different snapshots and thus failing the tests, so I had to play it a bit by ear here, but I hope this is working fine and is cleaned up now. I did test and things worked fine for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. frontend Issues related to the frontend kind/feature Categorizes issue or PR as related to a new feature. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use accent colors for clusters

6 participants