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

Add confirmation dialog before importing contacts #3759

Merged
merged 1 commit into from
Jan 30, 2024

Conversation

GretaD
Copy link
Contributor

@GretaD GretaD commented Dec 28, 2023

fixes #2788

Screenshot from 2024-01-29 21-30-09
Screenshot from 2024-01-29 21-30-24

@GretaD GretaD added the 2. developing Work in progress label Dec 28, 2023
@GretaD GretaD self-assigned this Dec 28, 2023
Copy link

codecov bot commented Dec 28, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (445f145) 2.51% compared to head (3e3fcd8) 0.00%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##              main   #3759      +/-   ##
==========================================
- Coverage     2.51%   0.00%   -2.52%     
- Complexity       0     261     +261     
==========================================
  Files           89      24      -65     
  Lines         4411     784    -3627     
  Branches      1126       0    -1126     
==========================================
- Hits           111       0     -111     
+ Misses        4180     784    -3396     
+ Partials       120       0     -120     

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

@GretaD GretaD force-pushed the feat/add-confirmation-dialog branch from 13ec9db to 858be11 Compare January 29, 2024 20:29
Copy link
Member

@st3iny st3iny left a comment

Choose a reason for hiding this comment

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

Tested and works.

I left some annotations about code that is not required any more.

src/components/ConfirmationDialog.vue Outdated Show resolved Hide resolved
src/components/ConfirmationDialog.vue Outdated Show resolved Hide resolved
src/components/ConfirmationDialog.vue Outdated Show resolved Hide resolved
src/components/ConfirmationDialog.vue Outdated Show resolved Hide resolved
@GretaD GretaD added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jan 30, 2024
@GretaD GretaD marked this pull request as ready for review January 30, 2024 11:42
@GretaD GretaD requested a review from st3iny January 30, 2024 13:36
Copy link
Member

@st3iny st3iny left a comment

Choose a reason for hiding this comment

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

Tested and works! Please squash.

@GretaD GretaD force-pushed the feat/add-confirmation-dialog branch from caf7218 to 3e3fcd8 Compare January 30, 2024 16:08
@GretaD GretaD enabled auto-merge January 30, 2024 16:10
@GretaD GretaD merged commit 57cce17 into main Jan 30, 2024
23 of 24 checks passed
@GretaD GretaD deleted the feat/add-confirmation-dialog branch January 30, 2024 17:37
// Redirect to the import page if the user confirmed
window.location = generateUrl(`/apps/contacts/import?file=${file.path}`)
} catch (e) {
// Do nothing if the user cancels
Copy link
Member

Choose a reason for hiding this comment

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

logging would be advisable here if anything but rejection is thrown

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i can do that with a follow up

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

Successfully merging this pull request may close these issues.

Confirm before importing contacts
3 participants