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

NAS-131790 / 25.04 / Defines SlideIn2CloseConfirmation interface for confirmation before closing slide-ins #11005

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

RehanY147
Copy link
Contributor

Changes:

Defines SlideIn2CloseConfirmation interface for confirmation before closing slide-ins

Testing:

Code review
Open the ssh-connection-form under Credentials -> Cloud Credentials and edit the form and close without saving. You should see a confirmation dialog asking if you're sure
Test on the ReplicationWizard under Data Protection and edit the form and close without saving. You shouldn't see a confirmation dialog and the dialog should close without a problem.

@RehanY147 RehanY147 requested a review from a team as a code owner November 10, 2024 02:23
@RehanY147 RehanY147 requested review from undsoft and removed request for a team November 10, 2024 02:23
@bugclerk bugclerk changed the title Defines SlideIn2CloseConfirmation interface for confirmation before closing slide-ins NAS-131790 / 25.04 / Defines SlideIn2CloseConfirmation interface for confirmation before closing slide-ins Nov 10, 2024
@bugclerk
Copy link
Contributor

Copy link

codecov bot commented Nov 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.02%. Comparing base (dd10db7) to head (b3c835b).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #11005      +/-   ##
==========================================
- Coverage   82.02%   82.02%   -0.01%     
==========================================
  Files        1616     1616              
  Lines       57004    57029      +25     
  Branches     5895     5898       +3     
==========================================
+ Hits        46760    46776      +16     
- Misses      10244    10253       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@undsoft undsoft left a comment

Choose a reason for hiding this comment

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

SshConnectionForm presents the dialog even when I click Save.

Also how about instead of making it a method/interface in parent class, we change it to something more explicit in slideInRef:

this.slideInRef.requireConfirmationWhen(() => { ... });

Copy link
Collaborator

@undsoft undsoft left a comment

Choose a reason for hiding this comment

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

Waiting for feedback to be addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants