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

Need to externalize list item type #199

Closed dpinn closed 5 years ago

dpinn commented 5 years ago

The Material.List module defines ul as:

ul :
    (Material.Msg m -> m)
    -> Index
    -> Material.Model m
    -> List (Property m)
    -> List (List.ListItem m)
    -> Html m

where List.ListItem refers to Internal.List.Implementation.ListItem.

berenddeboer commented 5 years ago

Why is this a problem? It's an internal type, no need for the user to bother with.

dpinn commented 5 years ago

I'd like to write functions to create my list items. The return type of such functions would reference an internal type, which just seems weird.

berenddeboer commented 5 years ago

OK let me know if this works for you. Given that ListItem are new, I appreciate any feedback. Let me know your use case!

I'm planning to use them everywhere, i.e. Drawer and Menu at this stage too, that's the only way I can see that would keep the user experience simple while still providing keyboard navigation and ripples.

berenddeboer commented 5 years ago

I did some quick experiment myself, and exposing ListItem does not give you the ability to write your own functions I believe. Not sure yet what needs to happen to make this work.

aforemny commented 5 years ago

Hi @berenddeboer, as far as I understood, the original issue was that you could not write the type myListItem : String -> Material.List.ListItem msg, but instead you had to write myListItem : String -> Material.Internal.List.ListItem msg.

So I think your commit ac7a42a solves that. Or am I missing something? @dpinn

aforemny commented 5 years ago

PS. @berenddeboer, have you considered making ListItem an opaque type?

type ListItem = ListItem { options : {- etc. -} }
berenddeboer commented 5 years ago

as far as I understood ... you had to write myListItem : String -> Material.Internal.List.ListItem msg.

Yes, if that's the case I fixed it.

But I did some kind of experiment if I could write a thing like li outside list's Implementation.elm. I couldn't get to work easily.

I.e. you want in your list not just Lists.li, but also SuperLists.li, where you try to do the same trick, i.e. returning a ListItem yourself.

berenddeboer commented 5 years ago

have you considered making ListItem an opaque type?

Actually the reverse :-) I was trying to expose the constructor to see if you write things like li in another module. But it said there was no ListItem constructor.

aforemny commented 5 years ago

@berenddeboer What would be a use-case to construct a ListItem directly? I am assuming that you could write any custom list items in terms of the functions exposed that return a ListItem such as li.

An example for a custom list item would be a list item that always sets a graphic icon and a text:

myListItem : String -> String -> ListItem msg
myListItem iconName label =
    Lists.li [ Lists.graphicIcon [] iconName, text label ]
dpinn commented 5 years ago

Hi @berenddeboer, as far as I understood, the original issue was that you could not write the type myListItem : String -> Material.List.ListItem msg, but instead you had to write myListItem : String -> Material.Internal.List.ListItem msg.

So I think your commit ac7a42a solves that. Or am I missing something? @dpinn

That's it exactly. I my case, I want to construct list items by mapping over a list of thingamebobs that I fetch from the server.

berenddeboer commented 5 years ago

A new piece of functionality is coming in #203: you can now include any HTML in a list if you prefix it with Lists.asListItem.