search

axmolengine / axmol

Axmol Engine – A Multi-platform Engine for Desktop, XBOX (UWP) and Mobile games. (A fork of Cocos2d-x-4.0)
https://axmol.dev
MIT License
929 stars 205 forks source link

Fix to allow CTRL+A key combination to work with multiple EditBox instances #2246

Closed rh101 closed 1 day ago

rh101 commented 2 days ago

Describe your changes

PR #2238 only worked with the first instance of an EditBox, because it seems that the same hotkey combination cannot be registered more than once globally, regardless of the unique address of the EditBox that it is bound to.

The changes in this PR register the CTRL+A key combination against the app Win32 window, and specifically checks if an EditBox is currently focused before applying the select-all text action. It also automatically de-registers the CTRL+A hotkey if all instances of EditBox are freed.

I'm not certain if this is the best solution to the issue, so if anyone sees any potential issues with this implementation, please leave feedback.

Issue ticket number and link

2236

Checklist before requesting a review

For each PR

For core/new feature PR

rh101 commented 1 day ago

Example of how it works: SelectAll

smilediver commented 1 day ago

I'm just worried that this could stop sending key events coming to the app through GLFW. For example, we hook GLFW to get events to the ImGui. So if you have an EditBox in scene, could this stop key events getting to the GLFW and ImGui?

rh101 commented 1 day ago

I'm just worried that this could stop sending key events coming to the app through GLFW. For example, we hook GLFW to get events to the ImGui. So if you have an EditBox in scene, could this stop key events getting to the GLFW and ImGui?

I just tested it with ImGui, and it does stop the CTRL+A hotkey combo from working in the ImGui fields, so this isn't a viable solution to the issue.

@halx99 I'll close this PR, and perhaps in the future we can find a better solution to this. It's not really a problem that users can't use CTRL+A to select all the text, just more of an inconvenience. Please revert #2238 as well, because that would have the same issue of preventing CTRL+A from working for anything besides EditBox.

rh101 commented 1 day ago

If developers do really need this, then perhaps we can make the behaviour in this PR optional, defaulting to disabled. The developer would need to explicitly enable it, knowing the side-effects of the CTRL+A hotkey not working for anything besides the EditBox instances.

If that is acceptable, then I can re-open this PR and add the necessary changes to toggle the functionality on or off at runtime.

smilediver commented 1 day ago

Would it be possible to subscribe for CTRL+A if edit box is actively editing text and unsubscribe if editing is done? That would still screw up ImGui, but at least only if edit box is active.

Edit: also, maybe it's a good idea to select all text automatically when edit box is activated?

rh101 commented 1 day ago

Would it be possible to subscribe for CTRL+A if edit box is actively editing text and unsubscribe if editing is done? That would still screw up ImGui, but at least only if edit box is active.

Possibly, by calling the RegisterHotKey on edit box focus, and then UnregisterHotKey when it is no longer focused, but yes, it will still mess with ImGui etc...

I'm looking into the possibility of just checking for a specific character with modifier on input prior to setting the text, and if CTRL+A is detected, then the text is selected, and the input is ignored so no "A" character is entered into the edit box.

Edit: also, maybe it's a good idea to select all text automatically when edit box is activated?

That's a good idea, and it should be something the developer can toggle. It should default to disabled to keep the same functionality. Can add it in a separate PR.

rh101 commented 1 day ago

@smilediver Much simpler solution in PR #2251 that doesn't block hotkeys to ImGui etc..