search

CSSG-Labs / PyNote

Text Editor Written in Python
GNU General Public License v3.0
3 stars 4 forks source link

17-quit-prompt + custom_prompt fix #34

Closed AdamLaine closed 2 years ago

AdamLaine commented 2 years ago

Issue #17 Finally, I am able to implement it. I had to fix, though, a little custom_prompt so as it will be able to react on empty (None) functions and so as it will be able to destroy itself with the mainloop. And also I have created save_and_close, which I don't know if it will be useful in the future (it was useful for me), maybe so as not to pollute the namespace I can put it inside the 'on_closing'?

AdamLaine commented 2 years ago

@PatrickBruso look also my answer in #33

AdamLaine commented 2 years ago

@mrHeavenli Thank you for the feedback. Could you also look at the #33 and tell what do you think and if you think that the changes are needed. Thank you

AdamLaine commented 2 years ago

Fellas, tell me if it is better

PatrickBruso commented 2 years ago

I'll take a look at this and see. I want to compare both the customprompt code and the messagebox code as I do see a benefit in being able to change the prompt buttons from yes/no to save/don't save.

mrHeavenli commented 2 years ago

@PatrickBruso another good thing about CustomPrompt, is that it is, as the name suggests, customisable, tkinter.messagebox is just a collection of common templates for prompts. Also, as its code is in another file, there aren't any if statements and the code becomes more readable. I'm sure it will become very useful in the future, however in this case, I do believe it's good to stick with the template provided by tkinter in the case of the quit prompt. (Sorry if the comment is a bit confusing)

PatrickBruso commented 2 years ago

@mrHeavenli I agree, after looking at the commit where it was used to quit I do think it is very useful. Also, using CustomPrompt in the main application is quite easy, although I still think the CustomPrompt file is fairly confusing for those without a lot of experience. Maybe we should put in some better documentation/examples in that file. I will approve this for now and I think later on we can consider switching all the prompts to CustomPrompt.