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

DragFloat, DragInt: Added io.ConfigDragClickToInputText to enable text input with a simple click? #3737

Open
1 task
ocornut opened this issue Jan 20, 2021 · 5 comments

Comments

@ocornut
Copy link
Owner

ocornut commented Jan 20, 2021

EDIT It's available, please see #3737 (comment) and try it.

I was suggested this change to DragXXX functions and would like to ask for feedback.

Making a simple-click (without movement) would turn the DragXXX into a text input, same as what currently happens with CTRL+Click. Click and drag would tweak the value as currently (with shift/alt being speed modifiers).

I suspect most people will be "looks good! / yeah!" about it but I would like to call on nitpicky minds to help me find undesirable side effects of doing this by default. Side-effects which could hinder user interactions in some circumstance, conflicts with existing code-base with specific logic, etc.

I already found one issue:

  • On devices using Touch Inputs mapped to Mouse and without a Keyboard (or with a Software Keyboard which is likely slow to popup), turning a DragXXX into a text input following a tap will be annoying and get in the way.
    Possible solution is to check for ImGuiConfigFlags_IsTouchScreen (however is a rather unknown config flag which I suspect no-one has ever used. If anything it should be reworded as a ImGuiBackendFlags_ value rather than ImGuiConfigFlags_). Another solution is to provide an explicit global setting to disable this feature.

Any others?

Trying it yourself?

I have pushed a commit (dc0f3d9) with the code commented-out:

// Experimental: simple click (without moving) turns Drag into an InputText
// FIXME: Currently polling ImGuiConfigFlags_IsTouchScreen, may either poll an hypothetical ImGuiBackendFlags_HasKeyboard and/or an explicit drag settings.
if (temp_input_allowed && !temp_input_is_active && !(g.IO.ConfigFlags & ImGuiConfigFlags_IsTouchScreen))
    if (g.ActiveId == id && hovered && g.IO.MouseReleased[0] && !IsMouseDragPastThreshold(0, g.IO.MouseDragThreshold * 0.5f))
    {
        g.NavInputId = id;
        temp_input_is_active = true;
        FocusableItemUnregister(window);
    }

Change #if 0 to #if 1 to test.
You may update or cherry-pick if you are on a recent version, otherwise simply paste the code in DragScalar() at the same location you see it in commit dc0f3d9.

Addenum: About mouse moving threshold

There is a threshold where we require the mouse to move after clicking before starting most drag operations, once the mouse gotten past this threshold it is always in unlocked state for this button.
Its default value is io.MouseDragThreshold = 6.0f (in theory it should be DPI-scaled but somehow I never heard a complain about it and I don't think anyone ever changed this threshold, haven't got around to scale all those thresholds).

Unlike most drag operations, DragXXX functions used an hard-coded threshold of 1.0f which is roughly equivalent to "any change in mouse position". I have now changed this value to io.MouseDragThreshold * 0.5f aka 3.0f with default settings, effectively slightly raising the threshold to start changing values in a DragXXX widget. I don't think it as any meaningful side-effect on using DragXXX widgets.

ocornut added a commit that referenced this issue Jan 20, 2021
…pressing it as a factor of default value + disabled experimental click-to-input on DragXXX functions. (#3737)
ocornut added a commit that referenced this issue Jan 20, 2021
…rning DragXXX widgets into text input with a simple mouse click-release (without moving). (#3737)

+ Offset ImGuiTableColumnFlags values.
@ocornut ocornut changed the title DragFloat, DragInt: mouse click (with no drag) to turn into text input? DragFloat, DragInt: Added io.ConfigDragClickToInputText to enable text input with a simple click? Jan 20, 2021
@ocornut
Copy link
Owner Author

ocornut commented Jan 20, 2021

Thought about it and I see no reason not putting this under a runtime flags, so I did:

in ImGuiIO:

bool ConfigDragClickToInputText = false; // [BETA] Enable turning DragXXX widgets into text input with a simple mouse click-release (without moving). Not desirable on devices without a keyboard.

I think we could eventually make it the default but for now until we get more feedback I'll make it opt-in instead of opt-out.

@elvissteinjr
Copy link

elvissteinjr commented Jan 29, 2021

I don't really have a hard opinion on this, but since this would affect me (or rather I'd turn it off and hope it stays configurable) I figured I could leave my thoughts on it.

In my case, the application can both be accessed on a normal desktop but also in VR. In the VR case there's no CTRL key to hold when clicking so I opted into enabling edit on right-click. Not looking to alter this behavior.
Clicking with no movement would be very difficult in the VR case anyways.

As far as user expectations go... wouldn't a double-click to edit make more sense? This would also be more consistent with how the Drag widgets behave.

But yeah, I think either will be fine for me. Just thought leaving some feedback is better than having nothing at all here.

Edit: Well poop, I misread and thought this was about Sliders for some reason. Sorry about that. Though both could be made to behave similarly, couldn't they? I only use Sliders in my project though.

@ocornut
Copy link
Owner Author

ocornut commented Oct 11, 2023

I've been considering enabling this by default io.ConfigDragClickToInputText = true in 1.90 but I haven't had enough feedback.

Please try this if you can!

Recap of raised points:

(1) in #2883 i mentioned "This would be problematic for DragInt mapped to enums using custom format strings."
--> Those could use ImGuiSliderFlags_NoInputs which was introduced in 1.78 after said comment.

(2) in #2883 i mentioned "This would be problematic on touch systems where keyboard inputs are not wired, or not efficient to use." and here "On devices using Touch Inputs mapped to Mouse and without a Keyboard (or with a Software Keyboard which is likely slow to popup), turning a DragXXX into a text input following a tap will be annoying and get in the way."
--> Touch-only applications could always change the default back to io.ConfigDragClickToInputText = false.
--> For hybrid desktop application, since we have access to MouseSource data since 1.89.5 we could decide to disable this behavior for TouchScreen/Pen, or introduce a different bool to set it separately.

(3) @elvissteinjr above mentioned the case of VR where the cursor is not stable. I would assume the solution would be the same as (2), manual disable or encourage VR users to call io.AddMouseSourceEvent().

(4) @elvissteinjr discussed Sliders. Unfortunately this currently only applies to Drags, where a typical click/touch is without side-effect. Our sliders historically have always been controlled with absolute position so initial click often modifies the value. Since 1.88 we improved sliders to make clicking over the knob have no-effect and instead add a slight offset over the absolute position. We could very well decide to redesign sliders or make that a global behavior option but that's another topic. I'm tempted to say Drags are generally preferable nowadays.

@Arugin
Copy link

Arugin commented Oct 11, 2023

Will try it out, thanks!

@Arugin
Copy link

Arugin commented Oct 14, 2023

I've tried it and it works great. I think it can be enabled by default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants