-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
feat(ui): allow terminate sync when sync start and terminate sync apps in list page #19718
base: master
Are you sure you want to change the base?
feat(ui): allow terminate sync when sync start and terminate sync apps in list page #19718
Conversation
Signed-off-by: linghaoSu <[email protected]>
🔴 Preview Environment stopped on BunnyshellSee: Environment Details | Pipeline Logs Available commands (reply to this comment):
|
✅ Preview Environment created on Bunnyshell but will not be auto-deployedSee: Environment Details Available commands (reply to this comment):
|
Subscribing 👀 |
It looks good! I would make the popup more specific to the sync. Maybe "Terminate Sync" instead of "Terminate Operation" because we lose context on what was the operation that we are terminating when the popup is open. |
Signed-off-by: linghaoSu <[email protected]>
@agaudreault Agreed, the description of the pop-up prompt for terminating the sync on the detail page is now a bit more precise |
Signed-off-by: linghaoSu <[email protected]>
Hi,@todaywasawesome @agaudreault I think this feature is ready to be reviewed, can you please help me review this pr? |
}); | ||
|
||
if (replaceChecked && containAppOfApps) { | ||
confirmSyncingAppOfApps(selectedAppOfApps, ctx, currentForm).then(confirmed => { |
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.
The naming is a bit confusing. ApplicationsTerminateSyncPanel
suggests it's only for termination of sync, so why do we confirm sync and check replace option? If we need to, shall we check prune as well?
@linghaoSu, I've asked for review in CNCF Slack. Can you rebase on the latest master and re-test, please? |
fix #18549
Add a button to terminate synchronisation more conveniently on the detail page.
cliped-new-720.mov
Add batch terminate sync panel on list page
cliped-480.mov
Checklist: