search

daniel-lerch / vocup

Vocabulary trainer.
https://www.microsoft.com/p/vocup/9n6w2h3qjqmm
GNU Affero General Public License v3.0
9 stars 3 forks source link

Fixes issue with wrong input focus and disabled state of special char button #54

Closed jdelange22 closed 2 months ago

jdelange22 commented 3 months ago

Fixes #51

Please note that PracticeDialog.cs is known as a binary file in the repo, probably due to incorrect file encoding. You might consider deleting this file and re-adding it using proper UTF-8 encoding.

daniel-lerch commented 2 months ago

@jdelange22 Sorry for the long delay. I changed the encoding of PracticeDialog.cs so that we get nice diff views on GitHub again. After that I rebased your branch on master to resolve conflicts. Finally, I did some refactoring to make the special char keyboard at least a little bit less messy. I hope you're fine with that. Changes like this are hard to explain in a code review.

Your pull request saved me a lot of time and you found cases I had always forgotten. Thank you very much!

jdelange22 commented 2 months ago

@jdelange22 Sorry for the long delay. I changed the encoding of PracticeDialog.cs so that we get nice diff views on GitHub again. After that I rebased your branch on master to resolve conflicts. Finally, I did some refactoring to make the special char keyboard at least a little bit less messy. I hope you're fine with that. Changes like this are hard to explain in a code review.

Your pull request saved me a lot of time and you found cases I had always forgotten. Thank you very much!

@daniel-lerch your changes in fact did broke the initial fix I made. When opening the practice window for the first time and opening the SpecialChar dialog, then the wrong textbox CAN get initialized. Upon opening the Dialog, the first textbox of the PracticeForm gets selected, even if it is not the one that is enabled for input. You removed the if (textbox != null && textbox.Enabled && !textbox.ReadOnly) from the TextBox_Enter(object sender, EventArgs e) method which fixed this.

daniel-lerch commented 2 months ago

Sorry, I oversaw this in the merge editor and probably didn't know why you added this. I introduced your fix again.