elm / html

Use HTML in Elm!
https://package.elm-lang.org/packages/elm/html/latest/
BSD 3-Clause "New" or "Revised" License
394 stars 99 forks source link

Add Attribute.map? #18

Closed jvoigtlaender closed 8 years ago

jvoigtlaender commented 8 years ago

I find it conceptually strange to have to write:

on "event-name" (someGivenDecoder |> Json.Decoder.map MyMessageConstructor)

More meaningful would be:

on "event-name" someGivenDecoder |> Attribute.map MyMessageConstructor

Because it is the attribute result to which I want to attach the MyMessageConstructor. I'm not conceptually trying to make a new decoder, so using Json.Decoder.map for what I want to express seems quite circumvent (when reading/understanding/explaining/teaching that code).

evancz commented 8 years ago

I'm not opposed to this, but I'd like to have a concrete example where it seems necessary.

jvoigtlaender commented 8 years ago

So does the following example qualify for re-opening?

Let's say I have already:

type Msg = ... | Click

view model =
    div []
    [ ...
    , button [ onClick Click ] [ text "Start" ]
    ]

Now I want to add an input to my view, and I want to get a message when a key goes up inside of it. So I enrich my Msg type:

type Msg = ... | Click | KeyUp Int

I also need to add the input to the view, and I have to make sure that an event handler is in place that sends a KeyUp message when the thing I'm waiting for happens. So:

view model =
    div []
    [ ...
    , button [ onClick Click ] [ text "Start" ]
    , input [ put-something-here ] [ ... ]
    ]

For the put-something-here, I'd currently have to do:

    , input [ on "keyup" (Html.Events.keyCode |> Json.Decoder.map KeyUp) ] [ ... ]

I think it would be better to instead be able to do:

    , input [ on "keyup" Html.Events.keyCode |> Attribute.map KeyUp ] [ ... ]

I shouldn't have to think about Json.Decoder here. After all, I'm trying to create an Attribute of a certain type, so Attribute.map is what I want to use. It also looks nicer in the code comparison, because it doesn't require the nested application.

evancz commented 8 years ago

I mean, put-something-here is not a concrete example. Obviously I get the scenario. The question is whether this comes up in real life for real users. If so, it should be easy to explain the scenario that came up.

jvoigtlaender commented 8 years ago

As hinted at in my first comment, it was a teaching scenario. The example was a made-up one: explaining ways of composing views, from different elements, with various event handlers, and how one has to map all results of the event handlers into the common Msg type. So, no, this was not from a concrete app, but preparing students to write their own apps, and trying to prepare them for situations they are not unlikely (in my assumption) to encounter. If that is too hypothetical for your taste, the issue is best left closed.

jvoigtlaender commented 8 years ago

@evancz, I just answered a question on the mailing list by somebody wanting to implement a game and registering mouse clicks relative to some element. I figure that counts as coming up in real life for a real user? The thread is here.

The code I felt reasonable to give that person under the current API was:

view model =
    div []
        [ Html.text (toString model)
        , div [ on "click" (Json.Decode.map Just offsetPosition) ]
            [ Element.toHtml (collage 100 100 [ filled red (circle 30) ]) ]
        ]

offsetPosition : Decoder ( Int, Int )
offsetPosition =
    Json.Decode.object2 (,)
        ("offsetX" := Json.Decode.int)
        ("offsetY" := Json.Decode.int)

What I would have preferred to give as code is replacing

        , div [ on "click" (Json.Decode.map Just offsetPosition) ]

by

        , div [ on "click" offsetPosition |> Html.Attributes.map Just ]

One reason is that under the assumption that offsetPosition were imported from some library (for example, elm-community/html-extra comes with several prepared special decoders), the user would currently have to do an extra import of Json.Decode (and know more about that module than just that decoders exist), while after the proposed addition of Attributes.map the user could entirely stay within the API of the elm-html package, which is probably something they are already familiar with and needs no conceptual excurse to Json.Decode.

Does that make sense?

jvoigtlaender commented 8 years ago

The other thread, on "mousemove", just reminded me of this one here.

@evancz, you had asked for "real world encounters" of what I had in my artificial example originally. So do you have thoughts on the encounter in my last comment above, and on the following newer one?

The latter, one could argue, may be influenced by me as instructor somehow, but the former not? In any case, influenced or not, are these cases where it is agreeable that having Attributes.map would be nice(r), and where it is not all just artificial?

evancz commented 8 years ago

Ah, I forgot as well. Will add to #53.

jvoigtlaender commented 8 years ago

Great!

gregziegan commented 7 years ago

@evancz real world example:

Building the elm-autocomplete API, we want to limit what users can do with attributes on certain Html nodes.

https://gist.github.com/evancz/bd916b9e4b48efbdb095b5fa9f09f6cf#file-autocomplete-elm-L78

There's no nice way to do this without an Attribute.map.

Since I cannot publish the API w/o using a Native module, this is a 🚫 blocker.

neilvyas commented 7 years ago

I have another example I stumbled into while trying out the elm architecture, specifically composition:

Suppose I have a Draggable component that has as its model only position information and whether or not it's currently being dragged. Now, suppose that my top-level model is a Dict of {id: String, draggable: Draggable and I have an input for creating/removing components by id that's controlled with an InputField String event.

Now, suppose I want to add an onClick listener to automatically populate the input field with the clicked draggable's id. Since id is not a field in the Draggable model, InputField String is a Msg and not a Draggable.Msg, so we don't have access to it in Draggable, and we can't add arbitrary event handlers to an existing view, especially with differing msg types, we have to write new view code. In this new top-level view code I use Draggable.onMouseDown to handle updating the shape's position, since I don't want to re-implement any of this code as well as needing Draggable.Msg rather than Msg messages, but in order to add the new input feature I need something like onClick (InputField id); note that these two attributes have different types: Attribute Draggable.Msg and Attribute Msg.

If Attribute.map : (a -> b) -> Attribute a -> Attribute b were implemented this would be easy, as I could simply map the Attribute Draggable.Msg into Attribute Msg as standard. As it is now I don't know how to do this without modifying Draggable's source code, and even worse, we would pollute Draggable with information only the top-level component needs (an id), as well as making it harder for future components to hold Draggables. Sorry if this doesn't make sense, needs actual code, or I'm approaching this issue the wrong way.