Skip to content

fix: Icon color#4836

Merged
zhanweizhang7 merged 1 commit intov2from
pr@v2@fix_icon_color
Mar 3, 2026
Merged

fix: Icon color#4836
zhanweizhang7 merged 1 commit intov2from
pr@v2@fix_icon_color

Conversation

@shaohuzhang1
Copy link
Contributor

fix: Icon color

@f2c-ci-robot
Copy link

f2c-ci-robot bot commented Mar 3, 2026

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@f2c-ci-robot
Copy link

f2c-ci-robot bot commented Mar 3, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

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

Details 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

h('rect', { width: '16', height: '15.9993', fill: 'currentColor' }),
]),
]),
],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The provided code looks clean and does not contain any immediate irregularities or potential issues. The color attribute values are set to '#3370FF' initially but then changed to 'currentColor', which is valid if the default color of elements should be used.

However, there's one suggestion:

Optimization Suggestion:

Ensure that the <defs> section at the bottom is not duplicated within other parts of the codebase. In this case, it appears only once where it defines the clip path for all icons in the map. It's always better to define definitions like SVG filters, gradients, etc., just once per file where they're needed to avoid duplication and improve performance. This can especially impact large projects with many SVG components.

no_tag = serializers.BooleanField(required=False,default=False, allow_null=True)
tag_exclude = serializers.BooleanField(required=False,default=False, allow_null=True)

def get_query_set(self):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code provided in the diff looks like it has duplicate field declarations for tag_ids and no_tag. This is unnecessary duplication, which should be removed to ensure consistency and possibly improve performance.

Here's the revised version of the code:

@@ -395,10 +395,6 @@ class Query(serializers.Serializer):
         tag_ids = serializers.ListField(
             child=serializers.UUIDField(),
             allow_null=True,
             required=False,
             allow_empty=True
         )
         no_tag = serializers.BooleanField(required=False, default=False, allow_null=True)
-        tag_exclude = serializers.BooleanField(required=False, default=False, allow_null=True)

Key Changes:

  1. Removed Duplicate Field Declarations:
    The lines - tag_ids = serializers.ListField(...) have been moved above the second occurrence of tag_exclude.

This improvement not only cleans up the code but also ensures that each field is declared exactly once, maintaining readability and reducing potential confusion or bugs.

@zhanweizhang7 zhanweizhang7 merged commit fc5c880 into v2 Mar 3, 2026
3 of 4 checks passed
@zhanweizhang7 zhanweizhang7 deleted the pr@v2@fix_icon_color branch March 3, 2026 09:32
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