-
Notifications
You must be signed in to change notification settings - Fork 615
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
Fix text input behavior #801
Conversation
There was a problem hiding this 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.
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? |
demo/glfw_opengl3/main.c
Outdated
@@ -8,6 +8,7 @@ | |||
#include <assert.h> | |||
#include <limits.h> | |||
#include <time.h> | |||
#include <ctype.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#include <ctype.h> |
demo/glfw_opengl3/main.c
Outdated
@@ -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))*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/*#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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
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.
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. |
This does a few things.
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.