david-vanderson / dvui

Other
346 stars 26 forks source link

NumberEntryWidget #105

Closed VisenDev closed 1 month ago

VisenDev commented 1 month ago

Closes #103

Supports all floating point and integer types with optional min and max threshholds.

iacore commented 1 month ago

Just by looking at the code, this seems annoying. If I type 1.2345 and delete 1, what happens? It doesn't let me edit the text?

Firefox solves this by not touching the text and giving validation error.

I confirmed this. Try typing 1.2e3.

iacore commented 1 month ago

I can't build this

❯ zig build
/home/user/computing/lib/zig/dvui/build.zig:90:40: error: no module named 'raylib' available within module root.@build
        const raylib_lib = try @import("raylib").addRaylib(b, target, optimize, .{ .raygui = true, .linux_display_backend = if (wayland) .Both else .X11 });
                                       ^~~~~~~~
referenced by:
    runBuild__anon_8820: /home/user/.zvm/0.13.0/lib/std/Build.zig:2117:37
    main: /home/user/.zvm/0.13.0/lib/compiler/build_runner.zig:301:29
    remaining reference traces hidden; use '-freference-trace' to see all reference traces
VisenDev commented 1 month ago

I can't build this

Yes that is from a bug that has.been fixed on main. Let me update the branch and that should be fixed

VisenDev commented 1 month ago

Firefox solves this by not touching the text and giving validation error.

That is another option. It just seemed easier if the entry widget just didn't let you enter invalid numbers.

Actually, perhaps the best way to handle this is preventing invalid numbers for integer types, and displaying a validation error for floating point numbers

The only problem with showing a validation error is that it does not seem to be possible to change a widgets dvui.Options after it has been inited and I only find out if the state is invalid after this

This means that is I used something like a red border to indicate invalid state - this display would always be lagged a frame behind.

david-vanderson commented 1 month ago

I like the idea of integers being filtered for only numbers, and floating point doing validation.

We should be able to do the validation and show the result in the same frame. Let me hack something up to give an idea.

Also, there's lots of widget stuff that you only need if you want to support having child widgets in yours. It's something I haven't done a great job of explaining (any ideas here would help).

david-vanderson commented 1 month ago

Does that help at all?

VisenDev commented 1 month ago

Does that help at all?

Yes, I updated the floating point code to display an error in this manner

david-vanderson commented 1 month ago

It's always tough to know how much logic to put into a widget. Filtering seems like something we might put into the TextEntryWidget, or at least provide a helper function for. I'm less sure about min/max, because in that case the app almost certainly wants some kind of custom validation message to provide to the user.

VisenDev commented 1 month ago

It's always tough to know how much logic to put into a widget. Filtering seems like something we might put into the TextEntryWidget, or at least provide a helper function for. I'm less sure about min/max, because in that case the app almost certainly wants some kind of custom validation message to provide to the user.

I am basing this off the raygui value box widget which is why I included min and max fields. I found the raygui widget to be very convenient to use which is why I wanted to implement something similar for dvui.

RAYGUIAPI int GuiValueBox(Rectangle bounds, const char *text, int *value, int minValue, int maxValue, bool editMode); // Value Box control, updates input text with numbers

I would much prefer filtering and parsing to be handled by the widget rather than the user having to manually handle this every time they need a number input

VisenDev commented 1 month ago

I am fine with the approach of just implementing this as a function also. However for integers I do think preventing entering numbers which are not valid ints of type T is very useful in lieu of having explicit min/max fields.

david-vanderson commented 1 month ago

I would much prefer filtering and parsing to be handled by the widget rather than the user having to manually handle this every time they need a number input

I agree, but a huge strength of immediate mode stuff is you can wrap it in functions. The filtering and parsing is annoying enough for us to provide a function for it.

Can you give more context for when the user wants min/max enforced in a text entry? It makes sense to me in the sliderEntry() but I'm having trouble understanding what is expected to happen if the user puts in a too low or too high value. Does it highlight in red? How would the user know what the problem was?

VisenDev commented 1 month ago

I'm having trouble understanding what is expected to happen if the user puts in a too low or too high value. Does it highlight in red? How would the user know what the problem was?

This is a fair point from a feedback perspective. It would probably be best to have the widget display the min/max bounds somehow. (Like some greyed out text inside the text entry showing (min-max))

Can you give more context for when the user wants min/max enforced in a text entry?

If you had to enter an rgb value that was restricted to 0-255 it would be nice for inputs to be limited to this so the user couldn't accidentally enter invalid numbers

Or for a normalized floating point number that had to be between 0.0 and 1.0

david-vanderson commented 1 month ago

What do you think of the api and usage of the prototype textEntryNumber()?

VisenDev commented 1 month ago

What do you think of the api and usage of the prototype textEntryNumber()?

Looks fine to me. The red error display also works for indicating to the user if invalid state has been entered.

We can remove NumberEntryWidget.zig then and just use this

david-vanderson commented 1 month ago

I very much appreciate the time and effort to work through this. I'm not sure we've got the right api yet, but this is a good step forward. Thank you!

VisenDev commented 1 month ago

I very much appreciate the time and effort to work through this. I'm not sure we've got the right api yet, but this is a good step forward. Thank you!

Of course, sorry if I've been bombarding you with PRs and issues lately.

david-vanderson commented 1 month ago

Not at all - this is great!

iacore commented 1 month ago

Actually, perhaps the best way to handle this is preventing invalid numbers for integer types, and displaying a validation error for floating point numbers

Have you considered that Zig std might accept 1_00_0? Or 000001.

david-vanderson commented 1 month ago

The "00001" example works. Stuff like "1000" hadn't occurred to me. It's pretty unusual for end users to type numbers like that though. I suppose we could add "" to the filter_chars list if somebody needed it.

VisenDev commented 1 month ago

I checked and zig fmt can parse numbers like 1_000000 so we should be able to add `` to the filter

iacore commented 1 month ago

but is 100_ a valid number? i feel like the current logic doesn't allow anyone to typing 100_0 out sequentially.

david-vanderson commented 1 month ago

Play around with it in the current examples under "Text Entry" - you should be able to enter invalid numbers and see a red outline. It doesn't allow "_" to be typed, because I would never imagine an end user typing that in. Makes sense for zig source code, but for GUI entry?

iacore commented 3 weeks ago

Makes sense