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 option to clear VSphere tags category #5940

Conversation

Waseem826
Copy link
Contributor

@Waseem826 Waseem826 commented May 4, 2023

What this PR does / why we need it:
Add an option to clear VSphere tags category so it doesn't get stuck when there are no tags.
Moreover, this PR also fixes an issue which was displaying removed labels even when creating cluster or machine deployment.

screenshot-localhost_8000-2023 05 04-17_44_57

Which issue(s) this PR fixes:

Fixes #5815

What type of PR is this?

/kind design

Special notes for your reviewer:

Does this PR introduce a user-facing change? Then add your Release Note here:

Add an option to clear VSphere tags category so it doesn't get stuck when there are no tags.

Documentation:

NONE

@kubermatic-bot kubermatic-bot added kind/bug Categorizes issue or PR as related to a bug. release-note Denotes a PR that will be considered when it comes time to generate release notes. docs/none Denotes a PR that doesn't need documentation (changes). dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note-none Denotes a PR that doesn't merit a release note. and removed release-note Denotes a PR that will be considered when it comes time to generate release notes. labels May 4, 2023
Copy link
Member

@ahmedwaleedmalik ahmedwaleedmalik left a comment

Choose a reason for hiding this comment

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

/approve

@kubermatic-bot kubermatic-bot added the lgtm Indicates that a PR is ready to be merged. label May 4, 2023
@kubermatic-bot
Copy link
Contributor

LGTM label has been added.

Git tree hash: dcf392dbb56b334c08d4d5275656e4ddbcd6ff41

@kubermatic-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahmedwaleedmalik, Waseem826

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

@kubermatic-bot kubermatic-bot merged commit 0a144e1 into kubermatic:main May 4, 2023
8 of 10 checks passed
@kubermatic-bot kubermatic-bot added this to the KKP 2.23 milestone May 4, 2023
@embik
Copy link
Member

embik commented May 4, 2023

@Waseem826 since this fixes a bug, can you maybe add a short release note so people looking for this bug being fixed see that we improved the UI? :)

@ahmedwaleedmalik
Copy link
Member

ahmedwaleedmalik commented May 4, 2023

since this fixes a bug

@embik as explained in the corresponding issue; this is not a bug. Seems like it's miscategorized here and I didn't really like the release note so asked @Waseem826 to omit it.

@embik
Copy link
Member

embik commented May 4, 2023

I disagree it's not a bug. It's just a visual bug as you explained and it doesn't have an impact on the ability to create a cluster from this state, but I don't see how that doesn't make it a frontend bug.

@ahmedwaleedmalik
Copy link
Member

It's just a visual bug as you explained

It isn't :) It's the way that component works as a default. We just "improved" its UX and functionality with this PR.

@embik
Copy link
Member

embik commented May 4, 2023

Okay, but we still changed something that is user-facing so I would like to suggest that the principal idea of "this should have a release note since it changes behaviour that the user is exposed to" still stands.

@ahmedwaleedmalik ahmedwaleedmalik added kind/chore Updating grunt tasks etc; no production code changes. kind/design Categorizes issue or PR as related to design. and removed kind/bug Categorizes issue or PR as related to a bug. kind/chore Updating grunt tasks etc; no production code changes. labels May 4, 2023
@kubermatic-bot kubermatic-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels May 4, 2023
@ahmedwaleedmalik
Copy link
Member

ahmedwaleedmalik commented May 4, 2023

@embik I have added the release note again.

@Waseem826 as discussed earlier, this PR is fine but we need a proper/generic solution at the component level to make sure that this behavior is the same for all usages of this component. Having behavioral drifts for components with similar functionalities is not ideal and is not UX friendly. Let's discuss it tomorrow or in the next sig-api-ui meeting.

@Waseem826 Waseem826 deleted the 5815-add-option-to-clear-vsphere-tags branch May 4, 2023 18:28
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. dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. docs/none Denotes a PR that doesn't need documentation (changes). kind/design Categorizes issue or PR as related to design. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot clear vSphere "Provider Tags" on machine wizard step
4 participants