Skip to content

Conversation

@sniok
Copy link
Contributor

@sniok sniok commented Sep 16, 2025

This PR fixes the icons test that checks that all icons are bundled properly
And updates all the latest icons to be included in the bundle

Related Issue

Fixes #3919 #3886

Steps to Test

  1. Start headlamp without internet
  2. Check that all the icons are visible

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Sep 16, 2025
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 16, 2025
@sniok sniok requested a review from Copilot September 16, 2025 13:13
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes the bundled icons test and updates icon definitions to ensure all icons are properly cached for offline use. It refactors the test to use a base directory parameter and updates the icon data with improved SVG paths and new icon definitions.

  • Fix the icons test by adding a baseDir parameter to the file filter function
  • Update all existing icon SVG paths with HTML entity encodings
  • Add new icon definitions and aliases to support additional UI elements

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
frontend/src/filesFilter/filesFilter.ts Add optional baseDir parameter to support custom base directories for file filtering
frontend/src/components/App/icons.ts Update icon SVG paths with HTML entities and add numerous new icon definitions and aliases
frontend/src/components/App/icons.test.ts Refactor test to use baseDir parameter and improve error reporting for missing icons

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

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.

Needs tests.

@sniok
Copy link
Contributor Author

sniok commented Sep 16, 2025

Needs tests.

what do you mean? this PR fixes the test that was broken

@joaquimrocha
Copy link
Contributor

Code looks fine.

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 19, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

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:

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

@k8s-ci-robot k8s-ci-robot merged commit daaa22b into kubernetes-sigs:main Sep 19, 2025
12 checks passed
@illume illume added this to the v0.36.0 milestone Sep 24, 2025
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. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Some icons cannot be Exist on a computer without a network

4 participants