-
Notifications
You must be signed in to change notification settings - Fork 620
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
ODC-7770: Remove perspective switcher if only one perspective is present #14762
base: main
Are you sure you want to change the base?
Conversation
@logonoff: This pull request references ODC-7770 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
appendTo: () => | ||
document.querySelector("[data-test-id='perspective-switcher-toggle']"), | ||
}} | ||
return activePerspective !== ACM_PERSPECTIVE_ID && perspectiveItems.length > 1 ? ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this logic work properly with ACM since we handle that perspective specially? If it's just the admin and acm perspectives, is the dropdown still hidden?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. FWIW, I have advocated for the removal of the special treatment of the "All Clusters" perspective since it causes confusion frequently when people expect the switcher to list all managed clusters, and further, ACM is introducing the ability to refer to "local-cluster" by any name the user chooses. Unsure what we would call this perspective though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's re-open this discussion since we're changing perspectives anyway... The cluster dropdown seems to create confusion, and we don't have any near term plans to add other clusters to the list. Can we make ACM a normal perspective? And what should it be called? cc @imjoyjean @kevinhatchoua
@logonoff Let's update the logic in this PR to handle ACM properly and make any ACM perspective changes in a follow-up. You should filter out the ACM perspective from the array before checking the length since ACM doesn't show up in this dropdown.
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
👍
/lgtm cancel @logonoff Sorry, I looked again at the code, and it seems ACM is already filtered out from |
This reverts commit 0dc0a13.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: logonoff, spadgett 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 |
@logonoff: all tests passed! Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
/cc @sanketpathak for QE review |
@invincibleJai: GitHub didn't allow me to request PR reviews from the following users: for, QE, review. Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
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. |
https://issues.redhat.com/browse/ODC-7770
before:
after:
/assign @vikram-raj @sanketpathak
/label px-approved docs-approved acknowledge-critical-fixes-only