Skip to content

Delete confirm update#60

Merged
math2001 merged 9 commits intomath2001:masterfrom
TerminalFi:delete-confirm-update
Jan 18, 2020
Merged

Delete confirm update#60
math2001 merged 9 commits intomath2001:masterfrom
TerminalFi:delete-confirm-update

Conversation

@TerminalFi
Copy link
Copy Markdown
Contributor

In reference to: #39

Purpose

In the current implementation, the confirm dialog presents all files into a single object on the quick look panel. This not only extends the height of each item, it makes it hard to visibly see the cancel button.

Changes

With the following changes, each file is broken into its own item on the quick look panel into its own row. It also adds the Confirm and Cancel options at the top of the list.

Confirm-Delete-Dialog

…ementation, definitely needs reviewed. However now, when a an item other than Confirm or Cancel is acted on, it is removed from the to be deleted list and then a new prompt is displayed for user confirmation.
Comment thread .gitignore
@math2001
Copy link
Copy Markdown
Owner

math2001 commented Jan 18, 2020

Oh that's a good idea! I really like it!! ♥️

I was wondering what it did when I selected just one file. I quickly looked at the code, and I thought that it would delete just that file. I took a bit of time and I realized that it removes just that item from the "to delete" list. I think your option is better.

But it needs to be clear, we can't live the user guessing. How about adding a message in the text under the Cancel thing? Maybe it could be something like Cancel the deletion of x items - select one of the item below remove it from the list. I don't really like the wording here, don't hesitate to use your own/propose some here if you're not sure.

Btw, we can delete files and folders, so we need to pay attention to that in the messages and in the code.

Also, all the code should be formatted using black. The current packages for black on sublime are either bloated (sublack) or less bloated but don't use the blackd server. I wrote a quick light plugin that just works out of the box, and is fast, but it's not really clean. I might fix it up and push it to GitHub (though I probably won't publish it to Package Control, because sublack already does everything my package will ever do).

If you don't want to install those plugins, just run black . before you commit, it'll fix up your code for you.

Damn, I just merged #59, so this is going to conflict if you want to make some changes. Let me know if you have any trouble with that.

Just to be clear, two more things that can need to be done:

  • format using black
  • improve the cancel message

Copy link
Copy Markdown
Owner

@math2001 math2001 left a comment

Choose a reason for hiding this comment

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

I really like that feature, but there are just a few things that I think can make the code a little bit better. And from what I gather from your other PR, you're after feedback, so I'm being 'bit peaky here 🙂

Comment thread FMcommands/delete.py Outdated
Comment thread FMcommands/delete.py Outdated
2. Updated the Confirm Title, Subtitle, Cancel Title, Subtitle
3. Updated the paths_to_display to utilize `os.path.basename` vs `path.split()`
@TerminalFi
Copy link
Copy Markdown
Contributor Author

Damn, I just merged #59, so this is going to conflict if you want to make some changes. Let me know if you have any trouble with that.

Just to be clear, two more things that can need to be done:

  • format using black
  • improve the cancel message

Done and done.

  • format using black
  • improve the cancel message

@TerminalFi
Copy link
Copy Markdown
Contributor Author

TerminalFi commented Jan 18, 2020

Just to show the change of wording

image

Copy link
Copy Markdown
Owner

@math2001 math2001 left a comment

Choose a reason for hiding this comment

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

That's so much easier to read ♥️

@math2001 math2001 merged commit 5f1b64c into math2001:master Jan 18, 2020
@math2001
Copy link
Copy Markdown
Owner

🎉 Thanks a bunch @TheSecEng, great idea!

@TerminalFi
Copy link
Copy Markdown
Contributor Author

@math2001 You are right, FileManager is amazing! Especially when combine with Deanishe's Alfred workflow for sublime projects.

image

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.

2 participants