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: clusterActionSlice: Refactor clusterActions into a slice #1446

Merged
merged 1 commit into from
Nov 13, 2023

Conversation

illume
Copy link
Collaborator

@illume illume commented Oct 10, 2023

  • Moved clusterAction related stuff from reducers/actions/sagas into clusterActionSlice.
  • Added some documentation.
  • Added some tests.

How to test?

  • Create or edit some yaml, and it should apply.
  • Try cancelling an action with the snackbar cancel buttons.

Part of #1159 epic.

@illume illume force-pushed the clusterAction-redux-toolkit branch from 1dc600c to e31f44e Compare October 10, 2023 09:49
@illume illume added the frontend Issues related to the frontend label Oct 10, 2023
@illume illume marked this pull request as draft October 10, 2023 09:59
@illume illume force-pushed the clusterAction-redux-toolkit branch 2 times, most recently from 7d98029 to a724f7d Compare October 10, 2023 15:30
@illume illume marked this pull request as ready for review October 10, 2023 16:31
@illume illume force-pushed the clusterAction-redux-toolkit branch from a724f7d to 655a44f Compare October 10, 2023 17:05
@illume illume force-pushed the clusterAction-redux-toolkit branch 2 times, most recently from 53d72ab to 42e55d2 Compare October 11, 2023 07:56
@illume illume force-pushed the clusterAction-redux-toolkit branch from 42e55d2 to a4b6183 Compare October 20, 2023 12:59
@illume
Copy link
Collaborator Author

illume commented Oct 20, 2023

Rebased against main.

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.

Why are sagas removed?
I have been thinking we should move request related code into sagas, to ensure they don't delay the UI.
I would like to understand how this code is analogous to what sagas provided. i.e. aren't we opening the door to a slower UI given we removed some things from sagas?

@illume
Copy link
Collaborator Author

illume commented Oct 25, 2023

Why are sagas removed?

Because redux-toolkit provides a way to do async stuff now, and it seems better to remove the saga dependency and use the one the redux folks decided on.

I have been thinking we should move request related code into sagas, to ensure they don't delay the UI. I would like to understand how this code is analogous to what sagas provided. i.e. aren't we opening the door to a slower UI given we removed some things from sagas?

I don't think it's slower. The requests are async and don't block the UI.

@illume illume force-pushed the clusterAction-redux-toolkit branch from a4b6183 to 7a386e5 Compare October 30, 2023 12:34
@illume
Copy link
Collaborator Author

illume commented Oct 30, 2023

Rebased against main.

@illume illume force-pushed the clusterAction-redux-toolkit branch from 7a386e5 to 55f976f Compare November 7, 2023 11:00
@illume
Copy link
Collaborator Author

illume commented Nov 7, 2023

Rebased against main.

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.

Code LGTM but testing it I have noticed that the Cancel snackbar doesn't go away when I click the cancel button.
How to reproduce:

  1. Go to a pod, click the delete button;
  2. After a second or two, click the delete button in the snackbar that appears. -> That snackbar should go away and the only the one about the having been cancelled should be on for a few more secs.

@illume
Copy link
Collaborator Author

illume commented Nov 13, 2023

I can't seem to reproduce it... Could you perhaps leave a gif showing how it's done?

This is what I was trying based on your description:
deleting a pod

@joaquimrocha
Copy link
Collaborator

@illume I think your video does show the issue.
See the main branch's behavior vs this branch's behavior.

@illume illume force-pushed the clusterAction-redux-toolkit branch from 55f976f to 0d75dbf Compare November 13, 2023 15:17
- Use new style redux toolkit, and not sagas.
- Added some documentation.
- Added some tests.

Signed-off-by: René Dudfield <[email protected]>
@illume illume force-pushed the clusterAction-redux-toolkit branch from 0d75dbf to e8148a0 Compare November 13, 2023 16:32
@illume
Copy link
Collaborator Author

illume commented Nov 13, 2023

Thanks.

I've updated it so the cancel snackbar leaves right away when the cancel button is pressed, like before.

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.

Works now. LGTM.

@joaquimrocha joaquimrocha merged commit 9ada050 into main Nov 13, 2023
6 checks passed
@joaquimrocha joaquimrocha deleted the clusterAction-redux-toolkit branch November 13, 2023 18:07
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants