Skip to content

Standardize the sync/suspend/edit buttons#4080

Merged
opudrovs merged 1 commit intomainfrom
WGE3374-standartize-sync-suspend-edit-buttons
Oct 31, 2023
Merged

Standardize the sync/suspend/edit buttons#4080
opudrovs merged 1 commit intomainfrom
WGE3374-standartize-sync-suspend-edit-buttons

Conversation

@opudrovs
Copy link
Copy Markdown
Contributor

@opudrovs opudrovs commented Oct 16, 2023

Part of weaveworks/weave-gitops-enterprise#3374

  • Replaced the Sync/Suspend/Resume controls, used in the SyncActions and CheckboxActions components, with the new Sync/Suspend/Resume controls (the SyncControl component).

  • Moved all components, related to syncing and suspending objects (existing SyncActions and CheckboxActions and new SyncControls and ResumeIcon), to the Sync folder.

  • Updated the related UI snapshot.

  • Added SyncControls to exports.

  • Moved custom actions to the start (left) of SyncControls buttons.

  • Re-arranged icons in IconType alphabetically (because it was hard to add new icons – to decide to which part of the list of icons or select cases to add new icon and to check if they already exist in the project, now it's somewhat easier).

Notes:

  • I've kept the original logic of creating sync/suspend handlers in general. Only added passing them to the new standartized SyncControls component and passing with/without source sync options (set with the new radio buttons) to the handlers.

  • We decided not to add Sync/Suspend notifications to enterprise for the time being. Notifications will be covered in the follow-up issues.

Screen recording of the new controls working in OSS:

Screen.Recording.2023-10-29.at.21.20.16.mp4

Design (there might be some differences between video and Figma design or design system, I tried to combine it in the most logical way):

The latest video with the design:
weaveworks/weave-gitops-enterprise#3374 (comment)
Figma:
https://www.figma.com/file/IVHnM9iyeFWpd11evtY8ux/Weave-GitOps?type=design&node-id=20086-71968&mode=design&t=46JWHoVfIwzQyShI-0

@opudrovs opudrovs added the type/enhancement New feature or request label Oct 16, 2023
@opudrovs opudrovs force-pushed the WGE3374-standartize-sync-suspend-edit-buttons branch 15 times, most recently from 8b79682 to c919c50 Compare October 23, 2023 08:09
@opudrovs opudrovs force-pushed the WGE3374-standartize-sync-suspend-edit-buttons branch 14 times, most recently from 41f1db5 to 77bf81d Compare October 27, 2023 11:59
@opudrovs
Copy link
Copy Markdown
Contributor Author

There is an enterprise counterpart to this PR:
weaveworks/weave-gitops-enterprise#3487
But tests in it just started failing after updating the OSS branch in it (probably because of this PR merged #4096 ), so, I hope it will be resolved after the following PR is merged:
weaveworks/weave-gitops-enterprise#3556
But the current PR can be reviewed independently.

@opudrovs
Copy link
Copy Markdown
Contributor Author

opudrovs commented Oct 30, 2023

@foot merged a small PR updating enterprise with the latest OSS main ✨ , so I will continue working on the enterprise counterpart of the current PR. But the current PR can be reviewed already.

@opudrovs
Copy link
Copy Markdown
Contributor Author

Add links to the latest video and Figma.

@opudrovs
Copy link
Copy Markdown
Contributor Author

Changed the format of my screen recording to MP4.

Copy link
Copy Markdown
Contributor

@jpellizzari jpellizzari left a comment

Choose a reason for hiding this comment

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

LGTM! Nice work

@opudrovs
Copy link
Copy Markdown
Contributor Author

@jpellizzari thank you! 🎉

Copy link
Copy Markdown
Contributor

@joshri joshri left a comment

Choose a reason for hiding this comment

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

This is awesome! Great code!

I noticed my sync requests in the ImageAutomations section are timing out, but I don't have enough faith in the objs on my cluster to say for sure if it's because of this PR - were you able to test img repos and update automations?

@opudrovs
Copy link
Copy Markdown
Contributor Author

opudrovs commented Oct 30, 2023

@joshri thank for testing it and notifying me about the issue! ✨

I don't think it is related to the current PR, because it should not affect the backend. But there was another PR merged in a few days ago which introduced some major changes to the sync/suspend backend code.

I will re-test my PR now with some ImageAutomations objects.

@joshri
Copy link
Copy Markdown
Contributor

joshri commented Oct 30, 2023

Yep - figured as much. I think this is safe to merge and we should address whatever's happening/not happening there in a follow up

@opudrovs
Copy link
Copy Markdown
Contributor Author

opudrovs commented Oct 30, 2023

@joshri tested several requests, have not been able to receive a request timeout or an error for iamge repos or image update automations, works fine for me.

Screenshot 2023-10-30 at 21 44 09 Screenshot 2023-10-30 at 21 44 17 Screenshot 2023-10-30 at 21 44 23 Screenshot 2023-10-30 at 21 44 37 Screenshot 2023-10-30 at 21 44 50 Screenshot 2023-10-30 at 21 45 03

@opudrovs
Copy link
Copy Markdown
Contributor Author

I only managed to reproduce request timeout (in most cases, but sometimes the request still succeeds) when trying to sync many objects, some of which are red or stuck in reconciling. But I see this issue in both main and current branches:

Screenshot 2023-10-30 at 22 21 36 Screenshot 2023-10-30 at 22 24 40

opudrovs added a commit that referenced this pull request Oct 30, 2023
…nd `CheckboxActions` components, with the new Sync/Suspend/Resume controls (the `SyncControl` component). (#4080).

* Create the new `SyncControls` component for Sync/Suspend/Resume controls.

* Move all components, related to syncing and suspending objects (existing `SyncActions` and `CheckboxActions` and new `SyncControls` and `ResumeIcon`), to the `Sync` folder.

* Update the related UI snapshot.

* Add `SyncControls` to exports.

* Move custom actions to the start (left) of `SyncControls` buttons.

* Re-arrange icons in `IconType` alphabetically.
@opudrovs opudrovs force-pushed the WGE3374-standartize-sync-suspend-edit-buttons branch from 98edb43 to 9bb7c2f Compare October 30, 2023 21:38
opudrovs added a commit that referenced this pull request Oct 30, 2023
…nd `CheckboxActions` components, with the new Sync/Suspend/Resume controls (the `SyncControl` component) (#4080)

* Create the new `SyncControls` component for Sync/Suspend/Resume controls.

* Move all components, related to syncing and suspending objects (existing `SyncActions` and `CheckboxActions` and new `SyncControls` and `ResumeIcon`), to the `Sync` folder.

* Update the related UI snapshot.

* Add `SyncControls` to exports.

* Move custom actions to the start (left) of `SyncControls` buttons.

* Re-arrange icons in `IconType` alphabetically.
@opudrovs opudrovs force-pushed the WGE3374-standartize-sync-suspend-edit-buttons branch from 9bb7c2f to 91db5c3 Compare October 30, 2023 22:44
opudrovs added a commit that referenced this pull request Oct 31, 2023
…nd `CheckboxActions` components, with the new Sync/Suspend/Resume controls (the `SyncControl` component) (#4080)

* Create the new `SyncControls` component for Sync/Suspend/Resume controls.

* Move all components, related to syncing and suspending objects (existing `SyncActions` and `CheckboxActions` and new `SyncControls` and `ResumeIcon`), to the `Sync` folder.

* Update the related UI snapshot.

* Add `SyncControls` to exports.

* Move custom actions to the start (left) of `SyncControls` buttons.

* Re-arrange icons in `IconType` alphabetically.
@opudrovs opudrovs force-pushed the WGE3374-standartize-sync-suspend-edit-buttons branch from 91db5c3 to fe336d6 Compare October 31, 2023 13:33
opudrovs added a commit that referenced this pull request Oct 31, 2023
…nd `CheckboxActions` components, with the new Sync/Suspend/Resume controls (the `SyncControl` component) (#4080)

* Create the new `SyncControls` component for Sync/Suspend/Resume controls.

* Move all components, related to syncing and suspending objects (existing `SyncActions` and `CheckboxActions` and new `SyncControls` and `ResumeIcon`), to the `Sync` folder.

* Update the related UI snapshot.

* Add `SyncControls` to exports.

* Move custom actions to the start (left) of `SyncControls` buttons.

* Re-arrange icons in `IconType` alphabetically.
@opudrovs opudrovs force-pushed the WGE3374-standartize-sync-suspend-edit-buttons branch from 56289e8 to dd4c5fc Compare October 31, 2023 13:45
opudrovs added a commit that referenced this pull request Oct 31, 2023
…nd `CheckboxActions` components, with the new Sync/Suspend/Resume controls (the `SyncControl` component) (#4080)

* Create the new `SyncControls` component for Sync/Suspend/Resume controls.

* Move all components, related to syncing and suspending objects (existing `SyncActions` and `CheckboxActions` and new `SyncControls` and `ResumeIcon`), to the `Sync` folder.

* Update the related UI snapshot.

* Add `SyncControls` to exports.

* Move custom actions to the start (left) of `SyncControls` buttons.

* Re-arrange icons in `IconType` alphabetically.
@opudrovs opudrovs force-pushed the WGE3374-standartize-sync-suspend-edit-buttons branch from dd4c5fc to 496ddb2 Compare October 31, 2023 13:55
opudrovs added a commit that referenced this pull request Oct 31, 2023
…nd `CheckboxActions` components, with the new Sync/Suspend/Resume controls (the `SyncControl` component) (#4080)

* Create the new `SyncControls` component for Sync/Suspend/Resume controls.

* Move all components, related to syncing and suspending objects (existing `SyncActions` and `CheckboxActions` and new `SyncControls` and `ResumeIcon`), to the `Sync` folder.

* Update the related UI snapshot.

* Add `SyncControls` to exports.

* Move custom actions to the start (left) of `SyncControls` buttons.

* Re-arrange icons in `IconType` alphabetically.
@opudrovs opudrovs force-pushed the WGE3374-standartize-sync-suspend-edit-buttons branch from 496ddb2 to b2d13d1 Compare October 31, 2023 14:57
…nd `CheckboxActions` components, with the new Sync/Suspend/Resume controls (the `SyncControl` component) (#4080)

* Create the new `SyncControls` component for Sync/Suspend/Resume controls.

* Move all components, related to syncing and suspending objects (existing `SyncActions` and `CheckboxActions` and new `SyncControls` and `ResumeIcon`), to the `Sync` folder.

* Update the related UI snapshot.

* Add `SyncControls` to exports.

* Move custom actions to the start (left) of `SyncControls` buttons.

* Re-arrange icons in `IconType` alphabetically.
@opudrovs opudrovs force-pushed the WGE3374-standartize-sync-suspend-edit-buttons branch from b2d13d1 to 859747a Compare October 31, 2023 15:44
@opudrovs
Copy link
Copy Markdown
Contributor Author

opudrovs commented Oct 31, 2023

Re-tested the current PR, works fine, so I am merging it. Will merge the enterprise part of this issue when enterprise is updated for the main OSS branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants