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

Textbox errors depending on cursor position upon submission #2037

Closed
fatteneder opened this issue Jun 9, 2022 · 12 comments · Fixed by #2041
Closed

Textbox errors depending on cursor position upon submission #2037

fatteneder opened this issue Jun 9, 2022 · 12 comments · Fixed by #2041

Comments

@fatteneder
Copy link
Contributor

fatteneder commented Jun 9, 2022

MWE

import Makie.MakieLayout: set!
using GLMakie
GLMakie.activate!()

fig = Figure()
idxbox = Textbox(fig[1,1], stored_string="1", tellwidth=false,
                 width=100, displayed_string="123")

prev_idx = parse(Int64, idxbox.stored_string[])
on(idxbox.stored_string) do s
  global prev_idx
  idx = parse(Int64, s)
  if idx == prev_idx
    idxbox.displayed_string[] = "$idx"
  else
    idx = max(idx,1)
    idx = min(idx,10)
    prev_idx = idx
    set!(idxbox, "$idx")
  end
end

display(fig)

The above gives you a Textbox which accepts only values in a range 1:10, e.g. typing into the box -1 and submitting it will reset the entered value to 1, submitting 14 will be reset to 10, values in between 1 and 10 are not altered.
Unfortunately, this does not work perfectly and throws the following error:

Stacktrace
Error in callback:
MethodError: Cannot `convert` an object of type Nothing to an object of type Vector{Point{2, Float32}}
Closest candidates are:
  convert(::Type{Array{T, N}}, ::FillArrays.Zeros{V, N}) where {T, V, N} at ~/.julia/packages/FillArrays/5Arin/src/FillArrays.jl:440
  convert(::Type{Array{T, N}}, ::StaticArrays.SizedArray{S, T, N, N, Array{T, N}}) where {S, T, N} at ~/.julia/packages/StaticArrays/58yy1/src/SizedArray.jl:118
  convert(::Type{Array{T, N}}, ::StaticArrays.SizedArray{S, T, N, M, TData} where {M, TData<:AbstractArray{T, M}}) where {T, S, N} at ~/.julia/packages/StaticArrays/58yy1/src/SizedArray.jl:112
  ...
Stacktrace:
  [1] setproperty!(x::Observable{Vector{Point{2, Float32}}}, f::Symbol, v::Nothing)
    @ Base ./Base.jl:43
  [2] setindex!(observable::Observable, val::Any)
    @ Observables ~/.julia/packages/Observables/ynr7h/src/Observables.jl:85
  [3] #15
    @ ~/.julia/packages/Observables/ynr7h/src/Observables.jl:393 [inlined]
  [4] (::Observables.var"#callback#13"{Observables.var"#15#16"{Makie.var"#1701#1718"{Label, Observable{Int64}}, Observable{Vector{Point{2, Float32}}}}, Tuple{Observable{Int64}, Observable{Vector{GeometryBasics.HyperRectangle{2, Float32}}}}})(x::Any)
    @ Observables ~/.julia/packages/Observables/ynr7h/src/Observables.jl:339
  [5] #invokelatest#2
    @ ./essentials.jl:716 [inlined]
  [6] invokelatest
    @ ./essentials.jl:714 [inlined]
  [7] notify
    @ ~/.julia/packages/Observables/ynr7h/src/Observables.jl:143 [inlined]
  [8] setindex!(observable::Observable, val::Any)
    @ Observables ~/.julia/packages/Observables/ynr7h/src/Observables.jl:86
--- the last 6 lines are repeated 1 more time ---
 [15] (::GridLayoutBase.var"#111#112"{Observable{Tuple{Union{Nothing, Float32}, Union{Nothing, Float32}}}})(sizeattrs::Tuple{Auto, Auto}, autosize::Tuple{Float32, Float32}, alignmode::Inside, protrusions::GridLayoutBase.RectSides{Float32}, tellsizeobservable::Tuple{Bool, Bool})
    @ GridLayoutBase ~/.julia/packages/GridLayoutBase/ZQ2p1/src/layoutobservables.jl:109
 [16] (::Observables.var"#callback#13"{GridLayoutBase.var"#111#112"{Observable{Tuple{Union{Nothing, Float32}, Union{Nothing, Float32}}}}, Tuple{Observable{Tuple{Union{Nothing, Float32, Auto, Fixed, Relative}, Union{Nothing, Float32, Auto, Fixed, Relative}}}, Observable{Tuple{Union{Nothing, Float32}, Union{Nothing, Float32}}}, Observable{Any}, Observable{GridLayoutBase.RectSides{Float32}}, Observable{Tuple{Bool, Bool}}}})(x::Any)
    @ Observables ~/.julia/packages/Observables/ynr7h/src/Observables.jl:339
 [17] #invokelatest#2
    @ ./essentials.jl:716 [inlined]
 [18] invokelatest
    @ ./essentials.jl:714 [inlined]
 [19] notify
    @ ~/.julia/packages/Observables/ynr7h/src/Observables.jl:143 [inlined]
 [20] setindex!(observable::Observable, val::Any)
    @ Observables ~/.julia/packages/Observables/ynr7h/src/Observables.jl:86
 [21] (::Makie.var"#1449#1451"{Label, Base.RefValue{GeometryBasics.HyperRectangle{2, Float32}}, MakieCore.Text{Tuple{String}}, LayoutObservables{GridLayout}})(#unused#::String, #unused#::Float32, #unused#::FreeTypeAbstraction.FTFont, #unused#::Float32, #unused#::Float32, padding::NTuple{4, Int64})
    @ Makie ~/wd/Makie.jl/src/makielayout/blocks/label.jl:26
 [22] (::Observables.var"#callback#13"{Makie.var"#1449#1451"{Label, Base.RefValue{GeometryBasics.HyperRectangle{2, Float32}}, MakieCore.Text{Tuple{String}}, LayoutObservables{GridLayout}}, Tuple{Observable{Any}, Observable{Float32}, Observable{FreeTypeAbstraction.FTFont}, Observable{Float32}, Observable{Float32}, Observable{Any}}})(x::Any)
    @ Observables ~/.julia/packages/Observables/ynr7h/src/Observables.jl:339
 [23] #invokelatest#2
    @ ./essentials.jl:716 [inlined]
 [24] invokelatest
    @ ./essentials.jl:714 [inlined]
 [25] notify
    @ ~/.julia/packages/Observables/ynr7h/src/Observables.jl:143 [inlined]
 [26] setindex!(observable::Observable, val::Any)
    @ Observables ~/.julia/packages/Observables/ynr7h/src/Observables.jl:86
 [27] #15
    @ ~/.julia/packages/Observables/ynr7h/src/Observables.jl:393 [inlined]
 [28] (::Observables.var"#callback#13"{Observables.var"#15#16"{Makie.var"#1009#1010"{DataType}, Observable{Any}}, Tuple{Observable{Any}}})(x::Any)
    @ Observables ~/.julia/packages/Observables/ynr7h/src/Observables.jl:339
 [29] #invokelatest#2
    @ ./essentials.jl:716 [inlined]
 [30] invokelatest
    @ ./essentials.jl:714 [inlined]
 [31] notify
    @ ~/.julia/packages/Observables/ynr7h/src/Observables.jl:143 [inlined]
 [32] setindex!(observable::Observable, val::Any)
    @ Observables ~/.julia/packages/Observables/ynr7h/src/Observables.jl:86
 [33] setproperty!
    @ ~/wd/Makie.jl/src/makielayout/blocks.jl:465 [inlined]
 [34] set!(tb::Textbox, string::String)
    @ Makie ~/wd/Makie.jl/src/makielayout/blocks/textbox.jl:310
 [35] (::var"#39#40")(idx::Int64)
    @ Main ~/wd/Makie.jl/mwe_bug.jl:27
 [36] #invokelatest#2
    @ ./essentials.jl:716 [inlined]
 [37] invokelatest
    @ ./essentials.jl:714 [inlined]
 [38] notify
    @ ~/.julia/packages/Observables/ynr7h/src/Observables.jl:143 [inlined]
 [39] setindex!(observable::Observable, val::Any)
    @ Observables ~/.julia/packages/Observables/ynr7h/src/Observables.jl:86
 [40] (::Makie.var"#1458#1474"{Slider, Observable{Any}})(i::Int64)
    @ Makie ~/wd/Makie.jl/src/makielayout/blocks/slider.jl:63
 [41] #invokelatest#2
    @ ./essentials.jl:716 [inlined]
 [42] invokelatest
    @ ./essentials.jl:714 [inlined]
 [43] notify
    @ ~/.julia/packages/Observables/ynr7h/src/Observables.jl:143 [inlined]
 [44] setindex!
    @ ~/.julia/packages/Observables/ynr7h/src/Observables.jl:86 [inlined]
 [45] setproperty!
    @ ~/wd/Makie.jl/src/makielayout/blocks.jl:465 [inlined]
 [46] set_close_to!(slider::Slider, value::Int64)
    @ Makie ~/wd/Makie.jl/src/makielayout/blocks/slider.jl:198
 [47] (::var"#37#38")(s::String)
    @ Main ~/wd/Makie.jl/mwe_bug.jl:20
 [48] #invokelatest#2
    @ ./essentials.jl:716 [inlined]
 [49] invokelatest
    @ ./essentials.jl:714 [inlined]
 [50] notify
    @ ~/.julia/packages/Observables/ynr7h/src/Observables.jl:143 [inlined]
 [51] setindex!(observable::Observable, val::Any)
    @ Observables ~/.julia/packages/Observables/ynr7h/src/Observables.jl:86
 [52] submit
    @ ~/wd/Makie.jl/src/makielayout/blocks/textbox.jl:195 [inlined]
 [53] (::Makie.var"#1710#1734"{Textbox, Makie.var"#cursor_backward#1733"{Observable{Int64}}, Makie.var"#cursor_forward#1732"{Textbox, Observable{Int64}}, Makie.var"#submit#1730"{Textbox, Observable{Bool}}, Makie.var"#removechar!#1728"{Textbox, Observable{Vector{Char}}, Observable{Int64}}, Observable{Int64}})(event::Makie.KeyEvent)
    @ Makie ~/wd/Makie.jl/src/makielayout/blocks/textbox.jl:228
 [54] #invokelatest#2
    @ ./essentials.jl:716 [inlined]
 [55] invokelatest
    @ ./essentials.jl:714 [inlined]
 [56] notify
    @ ~/.julia/packages/Observables/ynr7h/src/Observables.jl:143 [inlined]
 [57] setindex!
    @ ~/.julia/packages/Observables/ynr7h/src/Observables.jl:86 [inlined]
 [58] (::GLMakie.var"#keyoardbuttons#147"{Observable{Makie.KeyEvent}})(window::GLFW.Window, button::GLFW.Key, scancode::Int32, action::GLFW.Action, mods::Int32)
    @ GLMakie ~/wd/Makie.jl/GLMakie/src/events.jl:103
 [59] _KeyCallbackWrapper(window::GLFW.Window, key::GLFW.Key, scancode::Int32, action::GLFW.Action, mods::Int32)
    @ GLFW ~/.julia/packages/GLFW/BWxfF/src/callback.jl:43
 [60] PollEvents
    @ ~/.julia/packages/GLFW/BWxfF/src/glfw3.jl:620 [inlined]
 [61] pollevents
    @ ~/wd/Makie.jl/GLMakie/src/screen.jl:588 [inlined]
 [62] fps_renderloop(screen::GLMakie.Screen, framerate::Float64)
    @ GLMakie ~/wd/Makie.jl/GLMakie/src/rendering.jl:21
 [63] renderloop(screen::GLMakie.Screen; framerate::Float64)
    @ GLMakie ~/wd/Makie.jl/GLMakie/src/rendering.jl:48
 [64] renderloop(screen::GLMakie.Screen)
    @ GLMakie ~/wd/Makie.jl/GLMakie/src/rendering.jl:41

I played around with this and I think the problem has to do with where the cursor is positioned upon submitting the value. E.g. entering -1 and positioning the cursor

  • before or after the minus sign and submitting does not throw,
  • after the 1 and submitting throws the error.
    After some more tinkering I can say that the error is always thrown if the cursor is not positioned at the first or second place (counting from the left boundary).

The docs of Textbox say that displayed_string is for internal use.
Is this a bug or should we simply not use displayed_string? If the latter is the case, how could we work around this?

My bad, only stored_string is for internal use. Hence, I also use set! above to alter it.
But what about displayed_string? Are we not allowed to alter it or is this a bug?

@fatteneder
Copy link
Contributor Author

The problem happens on v0.17.3.

@fatteneder
Copy link
Contributor Author

The error might be because when we entered a value with the cursor sitting not at first or second place, and we reset the displayed string, then the only valid positions the cursor could go are the first two places.

@fatteneder
Copy link
Contributor Author

Just tested: One possible fix is to reset the cursor index upon submission, e.g. add cursorindex[] = 0 here:
https://github.com/JuliaPlots/Makie.jl/blob/24c8aae874d085da4afc512f5c673f48c216691d/src/makielayout/blocks/textbox.jl#L193-L197

If that sounds reasonable, I would upon a PR with this.

@jkrumbiegel
Copy link
Member

There's already a validation feature that you can use by the way, there's a validator or validation attribute that accepts a function.

@jkrumbiegel
Copy link
Member

@fatteneder
Copy link
Contributor Author

I am aware of that feature. However, I could not figure out how to use it to reset the displayed string if the entered value is out of bounds. Or am I missing something here?

The reason I want to do reset it is so that I can couple a Slider and a Textbox:

import Makie.MakieLayout: set!
using GLMakie
GLMakie.activate!()

fig = Figure()
slider = Slider(fig[1,1], range=1:10, startvalue=1)
idxbox = Textbox(fig[2,1], stored_string="1", tellwidth=false, width=100)

prev_idx = parse(Int64, idxbox.stored_string[])
on(idxbox.stored_string) do s
  global prev_idx
  idx = parse(Int64, s)
  if idx == prev_idx
    idxbox.displayed_string[] = "$idx"
    set_close_to!(slider, idx)
  else
    idx = max(idx,1)
    idx = min(idx,10)
    prev_idx = idx
    set!(idxbox, "$idx")
  end
end

on(slider.value) do idx
  global prev_idx
  if prev_idx != idx
    set!(idxbox, "$idx")
  end
end

display(fig)

This works now with the cursorindex[] = 0 addition.

@jkrumbiegel
Copy link
Member

It should not be able to store an invalid string at all, and reset the box after you set it, no? I've not used the textbox much after building it and it could be that what you want makes sense, though.

@fatteneder
Copy link
Contributor Author

It should not be able to store an invalid string at all, and reset the box after you set it, no?

Yes, reset the box if the value is out of bounds.

I tried with a custom validator. The code below does not store an invalid text, but does not reset content.

import Makie.MakieLayout: set!
using GLMakie
GLMakie.activate!()

function validate_inrange(str)
  idx = tryparse(Int64, str)
  if isnothing(idx)
    return false
  else
    return 1 <= idx <= 10
  end
end

fig = Figure()
idxbox = Textbox(fig[1,1], stored_string="1", tellwidth=false, width=100,
                 validator=validate_inrange)

prev_idx = parse(Int64, idxbox.stored_string[])
on(idxbox.stored_string) do s
  println(s)
end

display(fig)

@jkrumbiegel
Copy link
Member

aha I just tried it out and remembered there was also the attribute reset_on_defocus:

reset_on_defocus: Controls if the displayed text is reset to the stored text when defocusing the
       textbox without submitting. Default: false

If you set that to true, it resets when clicking somewhere else. What's still a bit buggy is that even on invalid inputs, submitting via enter makes the text gray and not red. That looks almost like it had submitted, which it didn't though so at least functionally it's correct. Just a visual inconsistency.

@jkrumbiegel
Copy link
Member

What's still a bit buggy is that even on invalid inputs, submitting via enter makes the text gray and not red.

Fixed that here #2041

@fatteneder
Copy link
Contributor Author

Oh, I must have missed that attribute, I am sorry.
Just tested and the fix now works as described.

Thanks for taking care of it.

@jkrumbiegel
Copy link
Member

Well it's easy to miss as there are no examples anywhere :) No worries!

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 a pull request may close this issue.

2 participants