Skip to content

Prevent clicking on slider knob from shifting its position #5328

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Prevent clicking on slider knob from shifting its position #5328

wants to merge 1 commit into from

Conversation

CamelCaseCam
Copy link

I saw that the v1.88 milestone has clicking on the slider knob changing its position (#1946) as one of the milestones. This is my first contribution, so I figured it would be a good place to start.

What I changed:

  • Added new slider flag ImGuiSliderFlags_NoChangeInsideHandle. Sliders behave exactly the same as before if this flag isn't passed.
  • When ImGuiSliderFlags_NoChangeInsideHandle is passed, sliders will ignore clicks inside the handle unless the handle is being dragged.
  • Added an extra checkbox to Widgets/Drag and Slider Flags in the demo window to demo ImGuiSliderFlags_NoChangeInsideHandle

See attached screen recording for example behaviour

2022-05-19.16-05-55.mp4

@ocornut
Copy link
Owner

ocornut commented Jun 20, 2022 •
edited
Loading

Thank you Cameron for your PR, much appreciated.

At first glance, I don't see value in adding the ImGuiSliderFlags_NoChangeInsideHandle flag: I feel the new behavior, if well implemented, would be always preferable.

  • In majority of cases, the handle is small enough that there's no legit scenario where old behavior is desirable.
  • In the case of using SliderInt() with small ranges and large handles, it is defacto pretty much the case that a "still" click doesn't alter the value.

I'll investigate this and the possibility of making that change but without the flag.

The PR itself isn't currently mergeable for a few reasons:

  • Using a static/global in the widget code. Generally g.ActiveIdClickOffset serves a similar purpose as what you added.
  • However I feel we could make an extra avoid to better handle feedback loops in the case where the slider size would change while holding mouse button.
  • Your last position compare is done on both axises whereas the slider is on a single axis, meaning a move in the other axis would trigger the edit. My mistake, apologies.

I will rework this soon.

ocornut added a commit that referenced this pull request Jun 20, 2022
@ocornut
Sliders: An initial click within the knob/grab doesn't shift its posi…
d3fd263 
…tion. (#1946, #5328) + Adjust default GrabMinSize.
@ocornut
Copy link
Owner

ocornut commented Jun 20, 2022

Reworked the fix as d3fd263, thank you for your help!

@ocornut ocornut closed this Jun 20, 2022
fangshun2004 added a commit to fangshun2004/imgui that referenced this pull request Jun 22, 2022
@fangshun2004
Squashed commit of the following:
340722d 
commit d3fd263
Author: ocornut <omarcornut@gmail.com>
Date:   Mon Jun 20 18:06:34 2022 +0200

    Sliders: An initial click within the knob/grab doesn't shift its position. (ocornut#1946, ocornut#5328) + Adjust default GrabMinSize.

commit f27af1b
Author: ocornut <omarcornut@gmail.com>
Date:   Mon Jun 20 16:44:49 2022 +0200

    Internals: SliderBehaviorT: Minor refactor, clearer 0.0/1.0 early out. Should be no-op from user's point of view.

    ScaleValueFromRatioT() had early 0.0/1.0 ratio tests, shifting most of function by one indent.

commit 90e8404
Author: omar <ocornut@users.noreply.github.com>
Date:   Mon Jun 20 15:35:48 2022 +0200

    Update README.md

commit 37a0785
Author: Rokas Kupstys <rokups@zoho.com>
Date:   Fri Jun 17 11:14:16 2022 +0300

    Nav: Fixed inability to cancel nav in modal popups. (ocornut#5400)

commit 07efd7c
Author: ocornut <omarcornut@gmail.com>
Date:   Wed Jun 15 15:58:26 2022 +0200

    Renamed IMGUI_DISABLE_METRICS_WINDOW to IMGUI_DISABLE_DEBUG_TOOLS.

commit 0857218
Author: ocornut <omarcornut@gmail.com>
Date:   Wed Jun 15 14:58:44 2022 +0200

    MovingWindow auto-cancelled if active id is stolen (instead of ill-defined bahavior + assert in docking).

    Followup to 27343ef

commit 27343ef
Author: ocornut <omarcornut@gmail.com>
Date:   Wed Jun 15 14:55:45 2022 +0200

    Nav, Focus: Changed SetKeyboardFocusHere() to not behave if a drag or window moving is in progress + move KeepAliveID() call from Scrollbar() to ScrollbarEx()

commit ddcff10
Author: ocornut <omarcornut@gmail.com>
Date:   Wed Jun 15 14:30:20 2022 +0200

    Settings: Fixed some SetNextWindowPos/SetNextWindowSize API calls not marking settings as dirty.

commit 6cac48d
Author: ocornut <omarcornut@gmail.com>
Date:   Wed Jun 15 11:51:19 2022 +0200

    Drag, Slider: rework slightly or CTRL+Click or SetKeyboardFocusHere() will show 1 change of active id in the log (rather than a set,clear,set sequence)

commit dd28500
Author: ocornut <omarcornut@gmail.com>
Date:   Tue Jun 14 19:06:44 2022 +0200

    Debug: Add more log. Reworked IMGUI_DEBUG_PRINT IMGUI_DEBUG_PRINTF. Added internal IsDragDropActive() helper.

    DebugLog() output to TTY by default.
    Amend 1d6e34f.

commit 2ed9e21
Author: ocornut <omarcornut@gmail.com>
Date:   Mon Jun 13 17:15:27 2022 +0200

    Nav, Internals: wrap changes to g.NavWindow into a helper function to help track/log changes.

    Amend 076d8fc. Eventually we should REALLY clean up the SetNavWindow SetNavID SetFocusID FocusWindow fiasco.

commit 1d6e34f
Author: ocornut <omarcornut@gmail.com>
Date:   Mon Jun 13 14:46:55 2022 +0200

    Debug: Added ShowDebugLogWindow().

    Internal: renamed old IMGUI_DEBUG_LOG() to IMGUI_DEBUG_PRINT().
    Amended once.

commit ec2c805
Author: Jack Knobel <jackknobel@users.noreply.github.com>
Date:   Sat Jun 11 20:52:27 2022 +1000

    Backends: support for unity builds for dx10/dx11/dx12 backends (ocornut#5387)

# Conflicts:
#	imgui.h
fangshun2004 added a commit to fangshun2004/imgui that referenced this pull request Jun 22, 2022
@fangshun2004
Squashed commit of the following:
f5d6937 
commit 9aae45e
Author: ocornut <omarcornut@gmail.com>
Date:   Tue Jun 21 17:38:27 2022 +0200

    Version 1.88

    (fix "Show Debug Log" checkbox in Metrics window)

commit d51e5d2
Author: ocornut <omarcornut@gmail.com>
Date:   Tue Jun 21 17:20:06 2022 +0200

    TabItem: revert support for SetNextItemOpen(true) at it creates too much ambiguity with p_open/close button vs Selected state. (ocornut#5262)

    Revert a small part of 4b97296.

commit c4b9101
Author: ocornut <omarcornut@gmail.com>
Date:   Tue Jun 21 17:11:51 2022 +0200

    TabBar: Tweak shrinking policy so that while resizing tabs that don't need shrinking keep their initial width more precisely.

    Has been the case before but adding support for SetNextItemWidth() ocornut#5262 made this more noticeable.

commit 4b97296
Author: ocornut <omarcornut@gmail.com>
Date:   Tue Jun 21 16:20:01 2022 +0200

    TabBar: TabItem() now reacts to SetNextItemWidth() and SetNextItemOpen(true). (ocornut#5262)

commit d3fd263
Author: ocornut <omarcornut@gmail.com>
Date:   Mon Jun 20 18:06:34 2022 +0200

    Sliders: An initial click within the knob/grab doesn't shift its position. (ocornut#1946, ocornut#5328) + Adjust default GrabMinSize.

commit f27af1b
Author: ocornut <omarcornut@gmail.com>
Date:   Mon Jun 20 16:44:49 2022 +0200

    Internals: SliderBehaviorT: Minor refactor, clearer 0.0/1.0 early out. Should be no-op from user's point of view.

    ScaleValueFromRatioT() had early 0.0/1.0 ratio tests, shifting most of function by one indent.

commit 90e8404
Author: omar <ocornut@users.noreply.github.com>
Date:   Mon Jun 20 15:35:48 2022 +0200

    Update README.md

commit 37a0785
Author: Rokas Kupstys <rokups@zoho.com>
Date:   Fri Jun 17 11:14:16 2022 +0300

    Nav: Fixed inability to cancel nav in modal popups. (ocornut#5400)

commit 07efd7c
Author: ocornut <omarcornut@gmail.com>
Date:   Wed Jun 15 15:58:26 2022 +0200

    Renamed IMGUI_DISABLE_METRICS_WINDOW to IMGUI_DISABLE_DEBUG_TOOLS.

commit 0857218
Author: ocornut <omarcornut@gmail.com>
Date:   Wed Jun 15 14:58:44 2022 +0200

    MovingWindow auto-cancelled if active id is stolen (instead of ill-defined bahavior + assert in docking).

    Followup to 27343ef

commit 27343ef
Author: ocornut <omarcornut@gmail.com>
Date:   Wed Jun 15 14:55:45 2022 +0200

    Nav, Focus: Changed SetKeyboardFocusHere() to not behave if a drag or window moving is in progress + move KeepAliveID() call from Scrollbar() to ScrollbarEx()

commit ddcff10
Author: ocornut <omarcornut@gmail.com>
Date:   Wed Jun 15 14:30:20 2022 +0200

    Settings: Fixed some SetNextWindowPos/SetNextWindowSize API calls not marking settings as dirty.

commit 6cac48d
Author: ocornut <omarcornut@gmail.com>
Date:   Wed Jun 15 11:51:19 2022 +0200

    Drag, Slider: rework slightly or CTRL+Click or SetKeyboardFocusHere() will show 1 change of active id in the log (rather than a set,clear,set sequence)

commit dd28500
Author: ocornut <omarcornut@gmail.com>
Date:   Tue Jun 14 19:06:44 2022 +0200

    Debug: Add more log. Reworked IMGUI_DEBUG_PRINT IMGUI_DEBUG_PRINTF. Added internal IsDragDropActive() helper.

    DebugLog() output to TTY by default.
    Amend 1d6e34f.

commit 2ed9e21
Author: ocornut <omarcornut@gmail.com>
Date:   Mon Jun 13 17:15:27 2022 +0200

    Nav, Internals: wrap changes to g.NavWindow into a helper function to help track/log changes.

    Amend 076d8fc. Eventually we should REALLY clean up the SetNavWindow SetNavID SetFocusID FocusWindow fiasco.

commit 1d6e34f
Author: ocornut <omarcornut@gmail.com>
Date:   Mon Jun 13 14:46:55 2022 +0200

    Debug: Added ShowDebugLogWindow().

    Internal: renamed old IMGUI_DEBUG_LOG() to IMGUI_DEBUG_PRINT().
    Amended once.

commit ec2c805
Author: Jack Knobel <jackknobel@users.noreply.github.com>
Date:   Sat Jun 11 20:52:27 2022 +1000

    Backends: support for unity builds for dx10/dx11/dx12 backends (ocornut#5387)

# Conflicts:
#	imgui.h
#	imgui_draw.cpp
#	imgui_tables.cpp
#	imgui_widgets.cpp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
sliders/drags
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@CamelCaseCam @ocornut