Skip to content

Fix text input behavior #801

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

Merged
merged 6 commits into from
Apr 7, 2025

Conversation

rswinkle
Copy link
Contributor

@rswinkle rswinkle commented Apr 5, 2025

This does a few things.

  1. It fixes the behavior of CTRL + LEFT/RIGHT to be both more standard and configurable. By default it now jumps to the beginning of words with whitespace being the separator, roughly in line with other text editors. Secondly the user can define NK_IS_WORD_BOUNDARY(c), a function style macro that takes a nk_rune aka nk_uint and returns true/1 if that character should be treated as a boundary, false otherwise. Of course I did all this before I realized/remembered that nk_text_editor.c is adapted from stb_textedit.h 1.8 in 2016. Since then user configurable word boundaries was added as well as several other fixes up through current 1.14. Sometime someone should review those changes and apply any we need or could benefit from.
  2. It fixes mouse scrolling. Like with the knobs, scrolling with the mousewheel was broken for multi-line text boxes. It might have been by design originally (based on 0 being passed for has_scrolling) but the behavior for the user was very inconsistent. Now it matches other subpanel behavior and user expectations. Horizontal scrolling is not handled as it's sort of a special case and there's no horizontal scrollbar even when there could/should be. Not a big deal either way imo.
  3. It makes the scrolling behavior follow the cursor in a consistent manner going both up and down. Before it would only scroll up if you went out of view, but it would jump down in increments every time you moved till it couldn't go further. Now going down works the same as up; the scrollbar doesn't move in either direction unless the cursor goes off the page.

In doing this work I saw another existing issue that is still there but is less important and will take more thinking. Clicking and dragging, the scrollbar follows the cursor enabling you to drag select the entire text. Theoretically. In reality you can't really drag upward past the top line of your current scroll position because the cursor is based on the mouse position and there's no way for the mouse to hover over a line that's not visible. The way to work around this is to just use the mouse wheel to scroll up while you hold the left button. Going down below the bottom can work and does in the Overview demo because the text box is not evenly divisible by the row height so you can actually hover just past the end of the last full line of text triggering the cursor to the next line and triggering the scrollbar to shift down.

I don't think this is a huge issue but if I had to fix it I might add a flag parameter to nk_textedit_locate_coord() to indicate it's a drag and and detect if you're in the top or bottom row and automatically jump up/down.

PS. I haven't really given up on the nk_image work but my idea to solve the backends that only support 1 to 1 image blitting problem led me to doing a lot of experimenting/research and it'll take me a few weeks at least before I have something somewhat polished and fast enough to try.

Copy link
Contributor

@RobLoach RobLoach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested this out a bit, and it seems to be working well. Nice work.

@rswinkle
Copy link
Contributor Author

rswinkle commented Apr 5, 2025

Thanks, let me know if there's anything I should add. I haven't bumped the version number yet and I should remove the changes to glfw_opengl3 from testing it but besides that anything you can think of?

@@ -8,6 +8,7 @@
#include <assert.h>
#include <limits.h>
#include <time.h>
#include <ctype.h>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#include <ctype.h>

@@ -21,6 +22,7 @@
#define NK_INCLUDE_DEFAULT_FONT
#define NK_IMPLEMENTATION
#define NK_GLFW_GL3_IMPLEMENTATION
/*#define NK_IS_WORD_BOUNDARY(c) (!isalnum(c))*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/*#define NK_IS_WORD_BOUNDARY(c) (!isalnum(c))*/

@@ -2,7 +2,7 @@
BIN = demo

# Flags
CFLAGS += -std=c89 -Wall -Wextra -pedantic
CFLAGS += -g -std=c89 -Wall -Wextra -pedantic
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm okay with this being here 🤷 you can remove if you want but meh

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll leave it. Honestly I'm always adding it to whatever demo I'm using to test my changes. It's already in 2 of the Makefiles (probably from me), and it doesn't really make sense not having it for non-release/production development demos anyway.

@rswinkle
Copy link
Contributor Author

rswinkle commented Apr 6, 2025

Wait, I just noticed something. Hold off on merging this.

I could see text input being a special case of always eating scroll if
you're active and have the visible cursor in there but I think it makes more
sense to match other subpanels and that's especially true when the text
box doesn't even have enough text to have a scrollbar; it's already a
bit odd that it eats scroll when that's true if you *are* hovering but
I think that's still understandable.

Also remove dead line of code.
@rswinkle
Copy link
Contributor Author

rswinkle commented Apr 6, 2025

Sorry about that. I noticed the scrolling-even-while-not-hovering issue and then realized I'd somehow forgotten to delete that equal sign as well. I swear I had already and had tested that case but clearly it was in one my earlier iterations. It should be good now.

@RobLoach RobLoach merged commit 8868050 into Immediate-Mode-UI:master Apr 7, 2025
1 check passed
@rswinkle rswinkle deleted the fix_text_input_behavior branch May 6, 2025 23:50
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