avh4 / elm-debug-controls

Easily build interactive UIs for complex data structures
https://avh4.github.io/elm-debug-controls/
BSD 3-Clause "New" or "Revised" License
24 stars 4 forks source link

Add lazy #2

Closed tesk9 closed 5 years ago

tesk9 commented 5 years ago

Adds lazy helper to support recursive controls.

recursive debug control

Adds an example view, although I'm not sure it adds much value. Would happily delete again if you'd like.

One 💭 I had was to maybe change Control from:

type Control a
    = Control
        { currentValue : () -> a
        , allValues : () -> List a
        , view : () -> ControlView a
        }

to

type Control a
    = Control (() -> 
            { currentValue : a
            , allValues : List a
            , view : ControlView a
            }
avh4 commented 5 years ago

Regarding changing Control to Control (() -> { ... } instead of having the functions inside of the record fields, that sounds okay, except the reason allValues was already a lazy function is because computing allValues can be very expensive, so I don't want it to be computed except where it's needed, so if you wanted to go that route, I'd want to see something like

type Control a
    = Control (() -> 
            { currentValue : a
            , allValues : () -> List a
            , view : ControlView a
            }

with allValues still being lazier than the rest of the fields.

avh4 commented 5 years ago

Looks good! The new example seems fine to keep.

Could you add some testing maybe similar to the new recursive type example along the lines of https://github.com/avh4/elm-debug-controls/blob/master/tests/Controls/ComplexChoiceTest.elm ?

And also, lazy will need some documentation.

tesk9 commented 5 years ago

@avh4 Turns out that allValues exploded with recursive values, which makes sense. How important is to have an allValues function, do you think? I don't see how allValues and lazy can both be exposed, you know?

I tried removing allValues, and I think it works (changes the API of list a bit tho). What do you think?

avh4 commented 5 years ago

Turns out that allValues exploded with recursive values

This means just that you can't call allValues on the control for the recursive type you defined, or that you couldn't actually implement lazy with the new types? If it's the former, I think it's best to keep it and just expect that it won't work for infinitely recursive controls.

(The purpose of allValues was for something like in a style guide where it could render the view with all the different values that could be selected so that you could quickly scroll through and visually ok them all without having to manually select each state from the interactive controls.)

tesk9 commented 5 years ago

Yeah, it's the former. I'll undo the removal then!

avh4 commented 5 years ago

Thanks! I'll try to remember to look at this tomorrow. (I think it should be good, though.)

avh4 commented 5 years ago

Thanks!

tesk9 commented 5 years ago

Thank you!!!