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

Add Kubernetes icon #86677

Merged
merged 2 commits into from
May 29, 2024
Merged

Add Kubernetes icon #86677

merged 2 commits into from
May 29, 2024

Conversation

sd2k
Copy link
Contributor

@sd2k sd2k commented Apr 22, 2024

What is this feature?

This commit adds a new icon for Kubernetes. I've just taken the SVG from the Grafana K8S plugin and plopped it in the unicons directory following the How to add a new icon instructions (removing width/height etc), but I'm unsure whether we should modify the SVG to instead have a single-colour version. I did have a quick go at this but it doesn't look quite right due to the lack of surrounding shape; perhaps everything needs inverting? I couldn't figure out how to do that though - happy to have another go if you think that'd look better.

In the below Storybook screenshot, the first (kubernetes) is the version I've included in this PR, while the second (kubernetes2) is the result of using the second path as a mask and removing the fill attributes.

image

Why do we need this feature?

Despite Grafana being used for lots of infra monitoring we don't have a Kubernetes icon. We do have a Docker one, so this feels like a natural extension.

Who is this feature for?

Plugin developers who want to be able to include a Kubernetes icon when using the Icon component from @grafana/ui.

Which issue(s) does this PR fix?:

N/A

Special notes for your reviewer:

I asked about this in Slack a few months ago but never got round to it until now!

Please check that:

  • It works as expected from a user's perspective.
  • If this is a pre-GA feature, it is behind a feature toggle.
  • The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

@sd2k sd2k requested review from a team as code owners April 22, 2024 11:27
@sd2k sd2k requested review from joshhunt, ashharrison90 and mckn and removed request for a team April 22, 2024 11:27
@grafana-delivery-bot grafana-delivery-bot bot added this to the 11.1.x milestone Apr 22, 2024
Copy link
Contributor

@mckn mckn left a comment

Choose a reason for hiding this comment

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

LGTM! But someone working more with the design system should probably do the approval.

@staton-hyse11
Copy link
Contributor

Would it be better to have this without the mask and only be a SVG? We will need the design asset to remain in parity with the code asset anyhow. If so, I can take the lead here and generate the SVG icon for this via Figma and attach it.

@sd2k
Copy link
Contributor Author

sd2k commented Apr 24, 2024

I think it might be better that way if it still looks correct without the mask yeah - I had a quick go at it but haven't really worked with SVGs before so didn't know what I was doing. Would really appreciate it if you could take a look @staton-hyse11!

@staton-hyse11
Copy link
Contributor

@sd2k I've tagged you in a Figma file for the SVGs. Leave your thoughts there and I'll follow up with the final assets in this PR.

@Aires-Grafana
Copy link

Is this already solved?
The "kubernetes2" is not a solution. I have the official SVG icon in Figma from Kubernetes.io in this Figma page.
If this is already solved, please disregard this message. Thanks!

@sd2k
Copy link
Contributor Author

sd2k commented Apr 25, 2024

I've updated the icon to the one provided in Slack. Storybook screenshot:

image

@jewbetcha
Copy link
Contributor

Really like that one @sd2k , k8s app team approves 👍

This commit adds a new icon for Kubernetes. I've just taken the SVG from
the Grafana K8S plugin and plopped it in the unicons directory following
the 'How to add a new icon' instructions (removing width/height etc), but
I'm unsure whether we should modify the SVG to instead have a single-colour
version. I did have a quick go at this but it doesn't look quite right
due to the lack of surrounding pentagon; perhaps everything needs inverting? I couldn't figure out how to do that though - happy to have another go
if you think that'd look better (see attached screenshot).
Copy link
Contributor

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 2 weeks if no further activity occurs. Please feel free to give a status update or ping for review. Thank you for your contributions!

@github-actions github-actions bot added the stale Issue with no recent activity label May 26, 2024
Copy link
Contributor

@staton-hyse11 staton-hyse11 left a comment

Choose a reason for hiding this comment

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

LGTM!

@github-actions github-actions bot removed the stale Issue with no recent activity label May 29, 2024
@Clarity-89 Clarity-89 added the no-changelog Skip including change in changelog/release notes label May 29, 2024
@sd2k sd2k merged commit 2455c0c into main May 29, 2024
22 checks passed
@sd2k sd2k deleted the add-kubernetes-icon branch May 29, 2024 08:50
bergquist added a commit that referenced this pull request May 29, 2024
* main: (25 commits)
  Update dependency @react-types/overlays to v3.8.7
  LogsTable: Fix default sort by time (#88398)
  Update dependency @react-types/menu to v3.9.9
  datatrails: in Components, `model.useState()`, instead of `model.state` (#88432)
  Update dependency @react-types/button to v3.9.4
  Alerting docs: adds template selector docs (#88412)
  Update dependency @playwright/test to v1.44.1
  Update dependency @kusto/monaco-kusto to v10.0.22
  Update dependency @grafana/faro-web-sdk to v1.7.3
  Alerting: Support `alertingDisableSendAlertsExternal` feature toggle (#88384)
  Update dependency @grafana/faro-web-sdk to v1.7.3
  Add navigation config for `grafana-csp-app` (#88360)
  Chore: create the `no-untranslated-literals` rule (#88271)
  Alerting: Update ListAlertRulesQuery to take a slice of RuleGroups (#88385)
  Update dependency @grafana/faro-core to v1.7.3
  Revamp tests for Add/Update Datasource (#88386)
  Update dependency @floating-ui/react to v0.26.16 (#88409)
  Update dependency cypress to v13.10.0 (#84140)
  Add Kubernetes icon (#86677)
  Update `make docs` procedure (#88404)
  ...
miguelmolina95 pushed a commit to miguelmolina95/grafana that referenced this pull request Jun 3, 2024
@kevinwcyu kevinwcyu modified the milestones: 11.1.x, 11.1.0 Jun 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/frontend no-changelog Skip including change in changelog/release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants