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

Improve how groups of reactive variables are managed #276

Open
PGimenez opened this issue Apr 29, 2024 · 9 comments
Open

Improve how groups of reactive variables are managed #276

PGimenez opened this issue Apr 29, 2024 · 9 comments
Assignees
Labels
enhancement New feature or request

Comments

@PGimenez
Copy link
Member

When the number of reactive variables grows large, managing them becomes difficult. We should have a way to group them to make the code more succint, and also make it easier to bring existing structures into an app. As of now we can use mixins or define a struct + its own Stipple.stipple_parse implementation, as seen in this Discord thread

Perhaps we could have a generic struct type built into Stipple so that users wouldn't need to define their own stipple_parse for each type. I've created a MWE for a ReactiveStruct abstract type. It works but there are two caveats:

  • The fields of the struct need to be in alphabetical order because stipple_parsercalls the constructor with arguments in this order. This could be solved with some more complicated logic
  • A handler cannot be attached to a field in a struct, you need to watch the whole struct.
using GenieFramework
@genietools

abstract type ReactiveStruct end

struct LayoutConfig <: ReactiveStruct
    left_drawer_open::Bool
    ministate::Bool
    selected_page::String
end

struct PageConfig <: ReactiveStruct
    icon::String
    title::String
end

mutable struct InputVars <: ReactiveStruct
    name::String
    age::Int
end

Stipple.stipple_parse(T::Type{ReactiveStruct}, d::Dict{String, Any}) = T([d[k] for k in sort(keys(d))])

@app begin
    @out layout = LayoutConfig(false, false, "")
    @out page = PageConfig("warning","Reactive structs")
    @in inputs = InputVars("John", 25)
    @in reset_name = false
    @onbutton reset_name begin
        @show inputs.name
        inputs.name = ""
        #changing a field in a struct does not trigger a UI update. We need to manually push the changes
        @push inputs
    end
    @onchange inputs begin
        println("An input changed")
    end
    # this does not work since inputs.name is not an observable
    #= @onchange inputs.name begin =#
    #=     println("Name changed") =#
    #= end =#
end

ui() = [h1("{{page.title}}"), p("Hello {{inputs.name}}!"), icon(R"page.icon"), textfield("Enter name", R"inputs.name"), button("Reset name", @click(:reset_name))]

@page("/", ui)
@PGimenez PGimenez added the enhancement New feature or request label Apr 29, 2024
@PGimenez PGimenez self-assigned this Apr 29, 2024
@hhaensel
Copy link
Member

hhaensel commented Apr 29, 2024

We could probably just modify the very first method for stipple_parse to achieve that without any type restriction

function stipple_parse(::Type{T}, value) where T
  if isstructtype(T)
    T([value[String(f)] for f in fieldnames(T)]...)
  else
    Base.parse(T, value)
  end
end

@hhaensel
Copy link
Member

We could even do this recursively:

T([stipple_parse(Ft, value[String(f)]) for (f, Ft) in zip(fieldnames(T), fieldtypes(T))]...)

@hhaensel
Copy link
Member

Finally, I'd also like to cover structs, that are defined via @kwdef and might be only partially defined:

function stipple_parse(::Type{T}, value) where T
  if isstructtype(T)
    ff = [String(f) for f in fieldnames(T)]
    kk = String.(keys(value))
    # if all fieldnames are present, generate the type directly from the fields
    if all(ff .∈ Ref(kk))
      T([stipple_parse(Ft, value[String(f)]) for (f, Ft) in zip(ff, fieldtypes(T))]...)
    # otherwise, try to generate it via kwargs, e.g. when the type is defined via @kwdef
    else
      T(; (Symbol(k) => v for (k,v) in value)...)
    end
  else
    Base.parse(T, value)
  end
end

@hhaensel
Copy link
Member

This will break any existing parse methods for struct types, but I'm currently not aware of any structtypes that we are using in Stipple via parse.
They should be overwritten in any case with stipple_parse, I think.

@hhaensel
Copy link
Member

@essenciary What do you think?

@svilupp
Copy link
Contributor

svilupp commented Apr 30, 2024

The fields of the struct need to be in alphabetical order because stipple_parsercalls the constructor with arguments in this order.

Not sure it helps here, but JSON3 StructTypes has OrderedStruct which ensures any arbitrary order of items is serialized/deserialized. But it’s probably not worth the extra complexity of doing the JSON intermediate representation.

@hhaensel
Copy link
Member

I use fieldnames(T) in order to bring the fields in the order in which the struct needs them. I tested the above proposal with nested structs. So this solution would release users to define a struct-specific stipple_parse.

@svilupp
Copy link
Contributor

svilupp commented May 5, 2024

Minor sidetrack -- where can I learn more about triggering updates with @push?

I searched everywhere in the repo and cannot find its definition to see how it works.

I'm referring to @PGimenez 's snippet above:

#changing a field in a struct does not trigger a UI update. We need to manually push the changes
@Push inputs

@hhaensel
Copy link
Member

hhaensel commented May 5, 2024

That's still poorly implemented.
I should convert it to a true macro, currently it's a replacement by the @onchange macro.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants