aforemny / elm-mdc

Elm port of the Material Components for the Web CSS/JS library
https://aforemny.github.io/elm-mdc
Apache License 2.0
354 stars 43 forks source link

A question about lifting messages #126

Open dpinn opened 6 years ago

dpinn commented 6 years ago

Just for my own edification: why have you used the idea of lifting messages (e.g.lift << Mdc) instead of using Html.map and Cmd.map in the Elm file that contains the component?

In Material.elm you wrote:

  • Material.Model and Material.Msg have to know your top-level message type Msg for technical reasons.

Can you give me some insight into those technical reasons? I've followed your practice, and I'd just like to understand the pattern better.

aforemny commented 6 years ago

Hi @dpinn, the reason for that is that a mdc component has to dispatch both internal as well as user messages. The idiom Html.map allows for only for either one of those types to be dispatched.

(In the following I will not parameterize Material.Msg m over m for clarity:) Consider Material.TextField, and your message type MyMsg = Mdc Material.Msg | Input String. If you write TextField.view Mdc "my-text-field" model.mdc [ TextField.onInput Input ] [], then obviously it will dispatch the user message Input. But deep inside Material.Msg there is a TextFieldMsg TextField.Focus for maintaining the focus state of the text field (styling state).

At the point when we are writing the type signature for TextField.view, we know that we have to dispatch Input : String -> MyMsg, as well as TextFieldMsg TextField.Focus : Material.Msg. Because of that we require a function that lifts Material.Msg -> MyMsg (this is the type signature of Mdc).

While I would not generally recommend this pattern in favor of Html.map or Cmd.map, I do not see any way around it in this instance.

Hope that helps. If anything remains unclear, let me know!

PS. In some instances, we are dispatching user messages from Material.update rather than view code. This is why Material.Model has to be parameterized over the user message as well. An example of that is Button.onClick where we want to dispatch the message only after the ripple animation has played.

dpinn commented 6 years ago

I'm still a little puzzled. In your Demo application, you have module Demo.Menus, in which the view method is like this:

view : (Msg m -> m) -> Page m -> Model m -> Html m
view lift page model =
    ...

...and in Demo.elm you call that function like this:

Demo.Menus.view MenuMsg page model.menus

If the Material.Model in Demo.Menus.elm was parameterized with Demo.Menus.Msg, could you not then change the signature of the view function to:

view : Page -> Model -> Html Msg
view lift page model =

... and call it like this?...

Demo.Menus.view page model.menus
    |> Html.map MenuMsg

... or does Material.Msg truly have to be parameterized with the 'top-level' type 'Msg'?

aforemny commented 6 years ago

Hi @dpinn,

you are right to be confused. If I were to write the demo today, I would not have used the lift-pattern for demo pages as well. I think it would turn out nicer if I did not.

However, for Material.Model m and Material.Msg m, we have to in fact keep the m.

So if we are considering Demo/Menus.elm, I would recommend writing it like this:

type alias Model =  -- not parameterized
    { mdc : Material.Model Msg  -- parameterized over this (=Demo.Menus.) Msg
    , …
    }

type Msg  -- not parameterized
    = Mdc (Material.Msg Msg)  -- parameterized over this (= Demo.Menus.) Msg
    = …

Then in ./demo/Demo.elm, you can use Html.map, etc. to do the lifting, like you suggest:

Demo.Menus.view page model.menus
    |> Html.map MenuMsg
dpinn commented 6 years ago

Thanks very much. I get the picture now. I think I'll stick with the idiom you used in the Demo, at least for now, since there's a lot of it in my code already. Sometime I might refactor it to the other way.

aforemny commented 6 years ago

Turns out that I cannot get rid of the lift pattern in the demo views after all, because the top level application provides wrapping functions like page.body to the demo pages. For the interface, refer to ./demo/Demo/Page.elm, for the implementation of that interface refer to ./demo/Demo.elm.

I require the lift pattern in views after all, because to the demo page view of resulting type Html Demo.Textfields.Msg wants to use a function page.body of resulting top level type Html Demo.Msg, and Demo.Msg = TextfieldsMsg Demo.TextFields.Msg | ….

dpinn commented 4 years ago

The Material.elm documentation says this in relation to the parameterisation of Model m and Msg m:

This takes as argument a reference to your top-level message type Msg.

What does it mean by top-level? If I have a nested structure, with models inside models, does the inner-most level have to be parameterised with the Msg type from the outer-most level?

berenddeboer commented 4 years ago

The nearest Msg. So if you have a model and msg per page and have a Material.Model Msg within them, the Msg here is the one for that page.

That's the simplest method. I hope this clarifies!

berenddeboer commented 4 years ago

I think we need to clarify this comment in Material.elm:

Material.Model` and `Material.Msg` have to know your top-level message type
    `Msg` for technical reasons.

That's true for an app with only one level of Msg. But for apps with a model and msg per page, this is confusing. Because in that case the top-level message is the msg at the level of the page model. So this needs to be written a bit more clear.

berenddeboer commented 4 years ago

And we need to rewrite the demo to make sure its pattern is not necessarily the best one.