Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add focusdelete parameter to text entry fields #1991

Closed
wants to merge 2 commits into from

Conversation

aBlueShadow
Copy link
Contributor

@aBlueShadow aBlueShadow commented Jul 2, 2023

This adds an optional boolean parameter to text entry fields. If true, the content of the text field is deleted when it gains focus. This is useful when it is very likely that the user wants to type in a completely new value instead of editing the old one, like numbers for example.

@daid
Copy link
Owner

daid commented Jul 3, 2023

There are two more common ways this is done:

Both methods are less "destructive" to the current value.

Also, following the API design of these controls, it shouldn't be a boolean parameter to the constructor but in the same way as setMultiline.

@aBlueShadow
Copy link
Contributor Author

The initial idea of this request came from the thoughts of how I could replace the fiddly ship window slider (almost impossible to set some specific values) with a text input field - without make the editing feel awkward.

focusdelete2.mp4

I guess both of your suggested variants would work in this case, as you usually won't edit the angle more than once.

@aBlueShadow
Copy link
Contributor Author

aBlueShadow commented Jul 4, 2023

Also, following the API design of these controls, it shouldn't be a boolean parameter to the constructor but in the same way as setMultiline.

As changing this and the otrher point in another commit won't make make sense (a diff on that PR would be larger than on master), I made a new PR: #1992

@daid
Copy link
Owner

daid commented Jul 5, 2023

Ok, then I'm closing this one, the other one is more to my liking.

Also, just a general bit of knowledge. boolean parameters to functions are considered bad API design, as you cannot see at the callee what is happening. function(True, False, False, True) isn't very obvious. A different way then these set functions would be to have flags, like the SDL2 API has https://wiki.libsdl.org/SDL2/SDL_CreateWindow (or the QT API)

Not that we need to go around and change all the current code, but it's good to keep in mind when writing code in general. I made quite a few "mistakes" while making EE, and learned a lot ;-)

@daid daid closed this Jul 5, 2023
@aBlueShadow aBlueShadow deleted the focusdelete branch January 14, 2024 19:19
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