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

InputText: Implement double/triple-click + drag word/line selection #8032

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ronak69
Copy link

@ronak69 ronak69 commented Oct 1, 2024

Select word/line on double/triple click was already implemented but
holding down the last click and dragging the mouse to incrementally
select was not.

To do this, `ImGuiInputTextState` keeps track of the current selection
mode (based on the number of mouse clicks) and the originating cursor
position over which the clicks happened. Later, on mouse moves, the
appropriate selection for mouse position and origin gets constructed.

Logic based on rxi/lite@53d555b.

Before:

imgui_selection_before.mp4

After:

imgui_selection_after.mp4

Some issues in double click selection:
Even if the cursor is already at a word boundary double-click will jump left. This patch fixes it. But I don't know why selection gets checked here. In my testing, the selection is never there and this assertion never fails.
diff --git a/imgui_widgets.cpp b/imgui_widgets.cpp
index ad1c1ea9a..282a35ae3 100644
--- a/imgui_widgets.cpp
+++ b/imgui_widgets.cpp
@@ -4638,7 +4638,9 @@ bool ImGui::InputTextEx(const char* label, const char* hint, char* buf, int buf_
                 // We always use the "Mac" word advance for double-click select vs CTRL+Right which use the platform dependent variant:
                 // FIXME: There are likely many ways to improve this behavior, but there's no "right" behavior (depends on use-case, software, OS)
                 const bool is_bol = (state->Stb->cursor == 0) || ImStb::STB_TEXTEDIT_GETCHAR(state, state->Stb->cursor - 1) == '\n';
-                if (STB_TEXT_HAS_SELECTION(state->Stb) || !is_bol)
+                const bool move_left = !ImStb::is_word_boundary_from_right(state, state->Stb->cursor);
+                IM_ASSERT(!STB_TEXT_HAS_SELECTION(state->Stb));
+                if (move_left && (STB_TEXT_HAS_SELECTION(state->Stb) || !is_bol))
                     state->OnKeyPressed(STB_TEXTEDIT_K_WORDLEFT);
                 //state->OnKeyPressed(STB_TEXTEDIT_K_WORDRIGHT | STB_TEXTEDIT_K_SHIFT);
                 if (!STB_TEXT_HAS_SELECTION(state->Stb))

See the difference it makes:

Before:

imgui_selection_1word_before.mp4

After:

imgui_selection_1word_after.mp4

Also, the is_bol check prevents selection if the clicks happen at the very last character. It should fallback to select the last word of the text instead.

Select word/line on double/triple click was already implemented but
holding down the last click and dragging the mouse to incrementally
select was not.

To do this, `ImGuiInputTextState` keeps track of the current selection
mode (based on the number of mouse clicks) and the originating cursor
position over which the clicks happened. Later, on mouse moves, the
appropriate selection for mouse position and origin gets constructed.
@ocornut
Copy link
Owner

ocornut commented Oct 3, 2024

Hello,

Thanks for your PR.
Due to the rather complex nature of the widget and this adding a little bit of complexity on top, I would like to come with a regression test that explains/exercise some of the edge cases.

This file has the existing tests for InputText():
https://github.com/ocornut/imgui_test_engine/blob/main/imgui_test_suite/imgui_tests_widgets_inputtext.cpp
Let me know if I can help getting acquainted with test suite.

The PR can use current value of IMGUI_VERSION_NUM which I would amend when merging code in main repo.

@ronak69
Copy link
Author

ronak69 commented Oct 3, 2024

Hello,

If the text buffer is "Hello world!", then to click on "w" I would need its on-screen coordinates. I found stb_textedit_find_charpos(). But that is in the implementation part of imstb_textedit.h, which I can't find a way to include/link to in test engine. Is there a way?

Otherwise, for now, should I manually measure the needed coordinates and hard code them in the test function?

@ocornut
Copy link
Owner

ocornut commented Oct 3, 2024

I guess you would call CalcTextSize() with the “Hello w” string subset.

@ocornut
Copy link
Owner

ocornut commented Oct 3, 2024

But i don’t think you would need to care about screen coordinates, only cursor positions.

@ronak69
Copy link
Author

ronak69 commented Oct 4, 2024

Wrote some fragile tests.

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

Successfully merging this pull request may close these issues.

2 participants