search

petalframework / petal_components

Phoenix + Live View HEEX Components
https://petal.build/components
MIT License
768 stars 84 forks source link

`.pc-accordion-item--last` class applied to all but the last accordion item. #229

Closed jpramassini closed 11 months ago

jpramassini commented 11 months ago

I was working on modifying the existing Accordion component (which is great, by the way) to have a fully rounded appearance instead of a rounded top and flat bottom. In doing so I was following the instructions outlined in the Modifying Components page to update the .pc-accordion-item--last class to add a rounded bottom to the last accordion selector item, which is what it seems like this selector is meant to apply to. For reference, the companion selector .pc-accordion-item__content-container--last works for styling the content box in the same way as expected, only applying the style to the last content box in the accordion. What actually happens with the accordion-item--last class is that it styles all buttons that aren't the last item instead.

It seems like this is due to the unless here: https://github.com/petalframework/petal_components/blob/ffa946c92f9a1a51ca12505b43651543cdf8e176/lib/petal_components/accordion.ex#L59C17-L63C19

Happy to get a fix in for this, but before doing so just want to confirm if this is intended behavior or not. Thanks!

jpramassini commented 11 months ago

After playing around with this a bit more locally, it seems like changing the behavior of this to try to do the rounding stuff (converting the unless to if, etc) doesn't do what I expected, so I think I probably just misunderstood the role this class. Closing, sorry!

mplatts commented 11 months ago

Hmm it does seem wrongly named. Seems like it should be called pc-accordion-item--all-except-last

jpramassini commented 11 months ago

Yeah, that name would probably make more sense given how it's applied. It would be good to change it to add clarity for people trying to do custom styling stuff, but I can also see how it could be a bit dangerous to rename if people are relying on that name for existing custom style overrides already.

nhobes commented 11 months ago

@jpramassini you're right about the naming convention, certainly more appropriately named something like @mplatts suggested e.g pc-accordion-item--all-except-last. I can update that now, hopefully not too many people have modified that and will discover this conversation. The current component also doesn't make it easy to modify the last item's corners to rounded. Perhaps it could be an attribute even. A PR would be welcome if you end up solving that.

jpramassini commented 11 months ago

Awesome, appreciate the name update! Not sure when I'll get to it, but if I end up messing around with it I'll definitely put up a PR 👍