search

bitcrowd / bitstyles_phoenix

A collection of Elixir phoenix helpers for bitstyles
ISC License
12 stars 0 forks source link

[#15] Add component: dropdown menu #56

Closed andreasknoepfle closed 2 years ago

andreasknoepfle commented 2 years ago
andreasknoepfle commented 2 years ago

whether separating the button+open/close-div from the menu would improve things (then the <:menu> slot would be called <:content> or <:wrapper> or so).

I disagree there! Like this, the alpine and live examples would simply be less clean again. I played around with this quite a while now 🙈
Also to add to this, normally you don't even need the menu slot, since the default simply works for you.

Also for anyone working with these slots for more than 5 minutes I think they will get that the tree structure is simply not a thing...

andreasknoepfle commented 2 years ago

🙏 for the review @maltoe

maltoe commented 2 years ago

the alpine and live examples would simply be less clean again

How so? Maybe I'm missing something here.

Also for anyone working with these slots for more than 5 minutes I think they will get that the tree structure is simply not a thing...

Yeah, might be true, but nonetheless it's a choice you're making about the API (and the API of future components): You said about the <dl>/<dt> component that the dt can't exists without the dl and hence should be a slot, which makes sense IMO - but with this idea taken to the extreme you would end up with high level components with a lot of slots that parametrize them, completely hiding the eventual DOM structure from the user. And I wonder if this is better than having a few more components and asking the user to write a bit more markup themselves.

But I'm reaaaally unsure about all of this ☝️ btw, maybe we should ask others for their opinions as well

andreasknoepfle commented 2 years ago

maybe we should ask others for their opinions as well

Yes. Although I think we can simply now go for it and change stuff later. I really want to go forward with the v1.0 release and see how it feels. I also put the last 3 months of thoughts into this :D, so it will be really hard to convince me of something else

maltoe commented 2 years ago

I also put the last 3 months of thoughts into this :D, so it will be really hard to convince me of something else

be aware of the Sunken cost fallacy 😃

i'm also excited for 1.0 and putting this in use - but all I'm saying is we're shaping an API here and should be mindful, and asking people coming from a React background for instance (where they essentially have solved/dealt with similar questions) might not hurt ;)

maltoe commented 2 years ago

alternative API here for reference:

<.ui_dropdown>
  <:button></:button> <!-- optional -->
  <.ui_dropdown_menu>
    <:option>...</:option>
  </.ui_dropdown_menu>
</.ui_dropdown>

Although I think we can simply now go for it and change stuff later

Also don't get me wrong, this is true of course ☝️ and the current API is wonderful and shiny and I'd love to use it instead of copying Darren's templates again and again and again :)

andreasknoepfle commented 2 years ago

Btw in your example, we end up with way more markdown for the default case. I think rarely someone will interact with the menu itself. So normal usecases will just be: 

<.ui_dropdown>
  <:button label="foo" />
  <:option>...</:option>
  <:option>...</:option>
</.ui_dropdown>

The only reason I exposed the menu is for adding attributes for JS/Live to it, which no one should need to do since it is already there. So I don't see why the alternative is better in any way except that it creates more HTML...

Also I can't make the button optional, since something needs to be on the label usually and a dropdown without a button makes no sense.

maltoe commented 2 years ago

So I don't see why the alternative is better in any way except that it creates more HTML...

As said, I'm not sure either that it is "better". Just conveying the the real DOM structure better. And separating the menu from the "dropdown behaviour" (i.e. a button that toggles visibility of an element).

Also I can't make the button optional, since something needs to be on the label usually and a dropdown without a button makes no sense.

True, sorry, somehow thought the button was optional and I should mark it as such. Nonsense of course.