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

enh: Replace confirm dialog for deletion with NcDialog #1663

Merged
merged 1 commit into from Dec 30, 2023

Conversation

Chartman123
Copy link
Collaborator

@Chartman123 Chartman123 commented Jun 30, 2023

Fixes #762

Before After
grafik grafik

@Chartman123 Chartman123 added enhancement New feature or request 2. developing Work in progress design Related to the design labels Jun 30, 2023
@Chartman123 Chartman123 added this to the 3.4 milestone Jun 30, 2023
@Chartman123 Chartman123 self-assigned this Jun 30, 2023
@Chartman123
Copy link
Collaborator Author

@susnux I had a conversation with @jotoeri about the issue behind this PR and about what he meant with "destructiveDialog". He told me that his intention was to have a standard way for this type of dialog in nextcloud-dialogs and that he already created two issues there.

nextcloud-libraries/nextcloud-dialogs#300
nextcloud-libraries/nextcloud-dialogs#301

What do you think about that?

@Chartman123 Chartman123 modified the milestones: 3.4, 3.5 Oct 13, 2023
@Chartman123 Chartman123 modified the milestones: 3.5, 4.0, 4.1 Nov 27, 2023
Copy link

codecov bot commented Dec 19, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (082e93b) 44.59% compared to head (c1e303c) 40.05%.
Report is 7 commits behind head on main.

❗ Current head c1e303c differs from pull request most recent head 5506552. Consider uploading reports for the commit 5506552 to get more accurate results

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1663      +/-   ##
============================================
- Coverage     44.59%   40.05%   -4.54%     
+ Complexity      657      562      -95     
============================================
  Files            58       55       -3     
  Lines          2592     2379     -213     
============================================
- Hits           1156      953     -203     
+ Misses         1436     1426      -10     

@Chartman123 Chartman123 added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Dec 19, 2023
@Chartman123 Chartman123 changed the title enh: Replace confirm dialog for deletion with modal enh: Replace confirm dialog for deletion with NcDialog Dec 19, 2023
@Chartman123 Chartman123 marked this pull request as ready for review December 19, 2023 21:34
@Chartman123 Chartman123 added the pending fix Waiting for a fix on one of our dependencies label Dec 22, 2023
Signed-off-by: Christian Hartmann <chris-hartmann@gmx.de>
@Chartman123 Chartman123 removed the pending fix Waiting for a fix on one of our dependencies label Dec 30, 2023
@Chartman123 Chartman123 merged commit 53f712d into main Dec 30, 2023
34 checks passed
@Chartman123 Chartman123 deleted the enh/confirm-deletion-dialogs branch December 30, 2023 09:39
@Chartman123
Copy link
Collaborator Author

@susnux now that nextcloud-libraries/nextcloud-dialogs#1297 is merged: should we adjust this for Forms to the new dialog?

@susnux
Copy link
Collaborator

susnux commented Apr 10, 2024

We can but not necessary - I am not sure whats cleaner. Meaning work to put into it and if the code outcome is cleaner than the current?

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 design Related to the design enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use destructiveDialog for Delete-Feedback
2 participants