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

frontend: Resource/CreateButton: Add cluster selector #2431

Merged
merged 1 commit into from
Oct 16, 2024
Merged

Conversation

illume
Copy link
Collaborator

@illume illume commented Oct 16, 2024

So users can select which cluster to apply the resource to.

Only shows the cluster selector if there is more than one
cluster being viewed.

For single cluster mode there is no change.

Note: depends on this being merged:

@illume illume changed the title frontend: Resource/CreateButton: Add cluster selector WIP: frontend: Resource/CreateButton: Add cluster selector Oct 16, 2024
@illume illume added frontend Issues related to the frontend multi Multi cluster aggregated view labels Oct 16, 2024
@illume illume marked this pull request as ready for review October 16, 2024 06:52
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Oct 16, 2024
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Oct 16, 2024
@illume illume changed the title WIP: frontend: Resource/CreateButton: Add cluster selector frontend: Resource/CreateButton: Add cluster selector Oct 16, 2024
Copy link
Collaborator

@joaquimrocha joaquimrocha left a comment

Choose a reason for hiding this comment

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

I think the cluster selector makes sense to be in the extraActions you created the other PR for, right?
As it's more discoverable for the user? Also, for now we may keep it single cluster selection, but it's be pretty cool to be able to select either the full cluster group or multiple individual clusters, so we could batch applies. But this can be added later.

@dosubot dosubot bot removed the lgtm This PR has been approved by a maintainer label Oct 16, 2024
So users can select which cluster to apply the resource to.

Only shows the cluster selector if there is more than one
cluster being viewed.

For single cluster mode there is no change.

Signed-off-by: René Dudfield <[email protected]>
Co-authored-by: René Dudfield <[email protected]>
Signed-off-by: Joaquim Rocha <[email protected]>
@illume illume mentioned this pull request Oct 16, 2024
24 tasks
@illume
Copy link
Collaborator Author

illume commented Oct 16, 2024

I think the cluster selector makes sense to be in the extraActions you created the other PR for, right?

Yeah [https://github.com//pull/2428/files#diff-bd17d08e06e757d92d123c96d6fe154071e41f2dd53bfd1a096743023d66e8d2R67
](in this "frontend: EditorDialog: Add optional actions property" PR), the actions prop was added which is used in this PR.

As it's more discoverable for the user? Also, for now we may keep it single cluster selection, but it's be pretty cool to be able to select either the full cluster group or multiple individual clusters, so we could batch applies. But this can be added later.

Ok, I added a task here for that: #1699 (comment)

@illume
Copy link
Collaborator Author

illume commented Oct 16, 2024

(rebased against main now that the #2428 is merged)

@illume illume requested a review from joaquimrocha October 16, 2024 09:30
@joaquimrocha
Copy link
Collaborator

I did misunderstand where the actions in this PR would end up. Clarified in DM. All good.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Oct 16, 2024
@illume illume merged commit 4752b32 into main Oct 16, 2024
18 checks passed
@illume illume deleted the cluster-selector branch October 16, 2024 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend Issues related to the frontend lgtm This PR has been approved by a maintainer multi Multi cluster aggregated view size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants