Desktop: Disable ok button at empty input

[coderrsid]

fixes #2799 and more such issue.

Explaination

This pull request changes the state of ok button to disabled in the PromptDialog.jsx whenever the input value in the dialog box is null or empty and i have tested this feature in various cases (New Notebook, Inserting a template) in which it disables the 'ok' button and the screenshots, GIF are shown below.

Screenshots and GIFs

Before

• New Notebook
  

• Insert Template
  

---

After

• New Notebook
  
  after adding title
  

• Insert Template
  

[coderrsid]

Label me @PackElend please!

[laurent22]

I think Bedwardly mentioned that somewhere but did you check where this prompt dialog is used, and does it always make sense to disable the OK button when there's no input?

[coderrsid]

Yeah, i checked, it is only used for some cases like set alarm, new notebook and select template and i tested for all and it always makes sense to disable the "OK" button is empty input. Below are the screenshots :

Set alarm

New Notebook

Select Template

[miciasto]

Set alarm

What about the Clear button?

[coderrsid]

What about the Clear button?

I think it's not the concern of this issue. But if you like i'll just make the change very quick and commit.

[coderrsid]

Like this @mic704b ?

[miciasto]

Yes, like that. I think it is virtually the same issue, is there ever any reason why the 'Clear' button should be enabled when there is nothing to clear?

[coderrsid]

No, there is every reason to be disabled. I thought it was not concern of this issue. But, as i see you agree. I'll just add the commit real quick.

[miciasto]

I thought it was not concern of this issue

Absolutely, you are correct, and as a rule we should only include changes directly on the issue in a PR. However in this rare case I think we can make an exception, with low risk the PR being rejected.

We could go and update the issue to be more general. Though, from my experience with this project, the goal is not to maintain an accurate issue history in github, but rather to have a solid app of high quality. So it would not be worth your or my time.

When we resolve issues, it is good to see if the same issue exists elsewhere, like we find here. Are there other buttons like this? If we find them, we might create a new issue, or we might fix it at the same time, depending on the similiarity and complexity of the fix.

[coderrsid]

Ohh sure mic, yeah the goal is to make a good application. The approach you just told is great. I am learning a great point here. But for now, i guess this PR is complete right.

[laurent22]

Actually you've introduced a change in behaviour by disabling the Clear button: previously, if an alarm is already set, you could clear the text input, then still press Clear and delete the alarm. But with that change you would disallow that action for no good reason. In other words, it's valid to clear an alarm even if the text input is empty.

When making this kind of change, you need to spend a bit of time to understand the existing behaviour, and decide whether it's safe to change it or not. In this case, you should leave the Clear button as it is.

[miciasto]

@laurent22 sorry, that is my fault.

Apologies @coderrsid, can you please undo the Clear button change.

[coderrsid]

It's fine @mic704b , you don't have to be sorry. 😄 We are all here to express our opinions. I'll just revert the last commit.

[laurent22]

No problem, I could have suggested something like this too but the key is, when making the change, to double-check it doesn't break anything.

[coderrsid]

@laurent22 I reverted the last commit( clear button disable ) as you suggested.

[laurent22]

Looks good, thanks @coderrsid

[coderrsid]

Thanks laurent 🤟😁