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

[NEW] Delete all Attachments / Files from a Channel /Chat #7383

Closed
wants to merge 4 commits into from

Conversation

danilomiranda
Copy link
Contributor

@RocketChat/core

Closes #6193

A new feature to remove all files from room with one click.
Plus an indicator of how much files are displayed

Plus an indicator of hoy much files are displayed
Copy link
Member

@geekgonecrazy geekgonecrazy left a comment

Choose a reason for hiding this comment

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

Thanks for opening this!

I know English is not likely your native language. But just a few changes to wording.

"You_wont_receive_email_notifications_because_you_have_not_verified_your_email": "You won't receive email notifications because you have not verified your email.",
"Your_email_has_been_queued_for_sending": "Your email has been queued for sending",
"Your_entry_has_been_deleted": "Your entry has been deleted.",
"Your_file_has_been_deleted": "Your file has been deleted.",
"Your_files_are_been_deleted": "Your files are been deleted.",
Copy link
Member

Choose a reason for hiding this comment

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

If this is for when its still deleting. You might want to change the wording here to:

Your files are being deleted.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this string is used after deletion is successful, so I think it should be

Your files have been deleted.

Copy link
Contributor Author

@danilomiranda danilomiranda Jul 4, 2017

Choose a reason for hiding this comment

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

No @gdelavald, we display the string during the delete process. I delete more than 800 files and the modal only close when all files are erased.

@@ -1687,6 +1687,7 @@
"Yes_unarchive_it": "Yes, unarchive it!",
"Yes_clear_all": "Yes, clear all!",
"Yes_delete_it": "Yes, delete it!",
"Yes_delete_all_it": "Yes, delete all it!",
Copy link
Member

Choose a reason for hiding this comment

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

I'd change the wording here to:

Yes, delete them all!

@@ -1716,10 +1717,12 @@
"You_should_name_it_to_easily_manage_your_integrations": "You should name it to easily manage your integrations.",
"You_will_not_be_able_to_recover": "You will not be able to recover this message!",
"You_will_not_be_able_to_recover_file": "You will not be able to recover this file!",
"You_will_not_be_able_to_recover_all_files": "You will not be able to recover all files!",
Copy link
Contributor

Choose a reason for hiding this comment

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

To fit with the message above (recover this file), I would suggest doing:

You will not be able to recover these files.

"You_wont_receive_email_notifications_because_you_have_not_verified_your_email": "You won't receive email notifications because you have not verified your email.",
"Your_email_has_been_queued_for_sending": "Your email has been queued for sending",
"Your_entry_has_been_deleted": "Your entry has been deleted.",
"Your_file_has_been_deleted": "Your file has been deleted.",
"Your_files_are_been_deleted": "Your files are being deleted.",
Copy link
Member

Choose a reason for hiding this comment

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

Can you update the variable name too? A little thing :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey, sorry to petpeeve on this, but since it's the success message after deleting multiple files, it should read

Your files have been deleted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah @geekgonecrazy , totally. I’ll do that asap :) thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gdelavald I see your point, but the message appears during the deletion. We can see that with a lot of files

Copy link
Member

Choose a reason for hiding this comment

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

@danilomiranda / @gdelavald I think the issue is that on large channels with lots of files the box will show while it's still deleting and won't go away until finished

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah it makes sense, but I would rather show a loading indicator while it's deleting and a successful message when it's done, I'm not sure what kind of logic that would need though.


self.files.map((file) => {
return removeFiles(file, msg);
});
Copy link
Member

Choose a reason for hiding this comment

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

.map is synchronous. So you could actually put that swal for success on multiple here.

As far as that goes... the swal for success really shouldn't be being shown until its actually finished. So might just move the swal below this whole block. And remove the text for TAPi18n.__('Your_files_are_being_deleted');

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The map function run very fast b/c the RocketChat.models.Uploads.remove is asynchronous. I tried to use the Meteor.wrapAsync without succes. move the swal to de end of the block doesn't delay the success message

@rodrigok
Copy link
Member

@danilomiranda Can you fix the conflict?

@danilomiranda
Copy link
Contributor Author

@rodrigok The @marceloschmidt is helping me with that.

@AmShaegar13
Copy link
Contributor

@danilomiranda May I help fixing the conflicts to get this one in?

@dhoeld
Copy link

dhoeld commented Jun 6, 2018

@danilomiranda Will be any further progress on the failing checks?

@ggazzo ggazzo requested a review from tassoevan August 7, 2018 22:30
@ggazzo ggazzo added the ui/ux label Aug 7, 2018
@ggazzo ggazzo added this to In progress in August/2018 via automation Aug 7, 2018
@ggazzo ggazzo added this to the 0.69.0 milestone Aug 7, 2018
@ggazzo
Copy link
Member

ggazzo commented Aug 14, 2018

@danilomiranda this issue was closed here #11236 sorry about that =/

@ggazzo ggazzo closed this Aug 14, 2018
August/2018 automation moved this from In progress to Done Aug 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
August/2018
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

7 participants