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

A question about lifting messages #126

Open
dpinn opened this issue Oct 5, 2018 · 9 comments
Open

A question about lifting messages #126

dpinn opened this issue Oct 5, 2018 · 9 comments

Comments

@dpinn
Copy link

dpinn commented Oct 5, 2018

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
Copy link
Owner

aforemny commented Oct 5, 2018

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
Copy link
Author

dpinn commented Oct 8, 2018

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
Copy link
Owner

aforemny commented Oct 8, 2018

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
Copy link
Author

dpinn commented Oct 8, 2018

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
Copy link
Owner

aforemny commented Oct 22, 2018

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
Copy link
Author

dpinn commented Jun 20, 2020

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
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants