search

imsnif / diskonaut

Terminal disk space navigator 🔭
MIT License
2.49k stars 66 forks source link

Feature: when trying to delete something while scanning, show a descriptive error message #32

Closed imsnif closed 4 years ago

imsnif commented 4 years ago

When we're still scanning, we don't want to let the user delete anything. Both for safety reasons (because things might be jumping around quite a little), and so that they can make an informed decision.

We do not give any indication for this though (only the key missing from the legend). It would be nice to have a modal appear when the user tries to delete in this mode. Something that would read "Sorry, deletion is only allowed once the scanning has completed". The modal should disappear after a few seconds.

mhdmhsni commented 4 years ago

Hey @imsnif, i want to give this one a try if you don't mind, but i have some questions beforehand:

  1. Do we need some sort of countdown indicator before disappearing the message? it may be a little distracting though. but i had to share my thought.

  2. One way of implementing such functionality can be defining a reusable WarningBox which takes an argument for example warning_message. (beside style changes and some other things.) but if you have any other suggestion (such as reusing currently available modals) please feel free to let me know.

  3. Should user be able to close the modal by pressing some key (Esc for example)? or closing the modal automatically after a few seconds is enough?

  4. and about the text truncation issue in small terminal windows, i want the WarningBox to be reusable for all other similar cases, so passing an array of possible texts instead of a single string value seems to be a good choice. but anyway i need to know your opinion or other solutions on this one.

imsnif commented 4 years ago 
1. Do we need some sort of countdown indicator before disappearing the message? it may be a little distracting though. but i had to share my thought.

Nah, I think it's fine to just have a Press <ESC> to dismiss line at the bottom of the modal.

2. One way of implementing  such functionality can be defining a reusable `WarningBox` which takes an argument for example `warning_message`. (beside style changes and some other things.) but if you have any other suggestion (such as reusing currently available modals) please feel free to let me know.

I would eventually really love to refactor all of the modals into one modal somehow. I haven't looked too closely at it, but I think that's the "right thing to do" in the end. It doesn't have to be part of this feature though - as you wish.

3. Should user be able to close the modal by pressing some key (`Esc` for example)? or closing the modal automatically after a few seconds is enough?

Yeah, I think would be best (but really, I think any key the user would press should dismiss it).

4. and about the text truncation issue in small terminal windows, i want the `WarningBox` to be reusable for all other similar cases, so passing an array of possible texts instead of a single string value seems to be a good choice. but anyway i need to know your opinion or other solutions on this one.

My preference is accounting for reusability as we need it. Like... do the easiest thing we can do right now to make it work and be readable, and then when we want to refactor stuff to combine functionality, we find the best way to do it. Because otherwise we might end up doing a lot of work that we'll realize we don't need when we get to adding more functionality (or combining functionalities) in the future. Makes sense?

mhdmhsni commented 4 years ago

My preference is accounting for reusability as we need it. Like... do the easiest thing we can do right now to make it work and be readable, and then when we want to refactor stuff to combine functionality, we find the best way to do it. Because otherwise we might end up doing a lot of work that we'll realize we don't need when we get to adding more functionality (or combining functionalities) in the future. Makes sense?

Yes! definitely makes sense. thanks for the explanations. :) I'm gonna work on this, if you don't mind of course.

imsnif commented 4 years ago

Of course! Let me know if you need anything.

mhdmhsni commented 4 years ago

Yaaay! solved the problem using your Event system! :)

Hi @imsnif, unfortunately i have a hard time implementing the auto close functionality (after n seconds). The core functionality of the WarningModal works fine and i have to write some tests for it to be completed but I think the issue here must be something obvious that i can't simply figure it out. I tried the async/await syntax and it had no progress, Neither spawning a new thread and moving the time calculation process into it didn't solve the problem (the scanning thread gets blocked in all cases). I thought maybe i have to define some new instructions (according to your instruction system) and play with it, but this one didn't work neither. So it seems to be a deadlock for me! :/ If there's anything that i've missed or if I'm mistaken about the whole process, please feel free to tell me your suggestion or anything. Thanks.