Closed oritwoen closed 11 months ago
Name | Edit | Preview | Latest Commit |
---|---|---|---|
ui | Edit on Studio ↗︎ | View Live Preview | 5485b1ce1a73d08dcf0b10c3d75dc2d5c1662959 |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
Name | Status | Preview | Updated (UTC) |
---|---|---|---|
ui | 🔄 Building (Inspect) | Jun 28, 2023 8:10pm |
@oritwoen is attempting to deploy a commit to the Nuxt Labs Team on Vercel.
A member of the Team first needs to authorize it.
@oritwoen, nice additions! ❤️
@Haythamasalama I corrected the types in the code.
@benjamincanac Right multiple
fits better. From what you wrote I understood that the default multiple
is supposed to be false
.
This creates a small breaking change for users who already used Accordion
in their somewhere. But actually Accordion
I generally associate with one element open at a time and that fits the default mode. Unless I misunderstood your message?
I have updated the code and documentation based on the above issues.
For the items objects, I left the closeOthers
variable as it was because there, in my opinion, it fits appropriately - it closes others if the multiple
mode is set to true.
There was also the issue of transition
for "one at a time mode" which I wanted to do based on the max-height
animation but apparently you resolved that issue very efficiently here: https://github.com/nuxtlabs/ui/commit/f3c6f83232c2c75dbbbef82d98e6b66d48ced263
This creates a small breaking change for users who already used Accordion in their somewhere. But actually Accordion I generally associate with one element open at a time and that fits the default mode. Unless I misunderstood your message?
So far it is in Edge so we are find to change the behaviour before it becomes stable.
@oritwoen It's ok to let multiple
to false
as indeed this component hasn't been published yet.
There is still a timing issue with the transition when multiple
is false, we can see a slight shift between the open of the new item and the close of the other above or below.
Why not to let closeOthers
in the items as it might be useful to some!
Also, this behaviour doesn't work when overriding the button in the default slot. How can we achieve that without passing the closeAll
function and asking users to bind it to their button?
@benjamincanac @oritwoen
I'm wondering if we can rename multiple
to multiple-open
?
I think it would be clearer to indicate their association with opening and closing.
@oritwoen I am unable to push to this PR to assist you in achieving it. However, I would like to contribute by adding something
1- To solve the issue with the closeAll
function and slot:
In the Accordion
component, pass :close-all="closeAll"
to the slot:
<HDisclosureButton :ref="() => updateClosesRefs(close, index)" as="template" :disabled="item.disabled">
<slot :item="item" :index="index" :open="open" :close="close" :close-all="closeAll">
// ..
In the AccordionExampleDefaultSlot
, add closeAll
and @click="closeAll(index)"
:
<UAccordion :items="items" :ui="{ wrapper: 'flex flex-col w-full' }">
<template #default="{ item, index, open , closeAll }">
<UButton //.... @click="closeAll(index)">
I have tested it, and it works correctly.
2- It is unnecessary to add as HTMLElement in (el as HTMLElement).
function onLeave (el: HTMLElement, done) {
// (el as HTMLElement).addEventListener('transitionend', done, { once: true })
el.addEventListener('transitionend', done, { once: true })
}
The el
parameter is already declared as type HTMLElement, so the type assertion is not required.
@Haythamasalama That was my point, I don't like the idea of asking users to manually assign the function themself when overriding the default slot. I'm looking for another way to achieve this!
I've updated the behaviour with a child component that emits when the open
changes. This way, there is no click function to bind on the buttons. Let me know what you think 😊
I've updated the behaviour with a child component that emits when the
open
changes. This way, there is no click function to bind on the buttons. Let me know what you think 😊
This way it's very perfect ❤️
But I still noticed a slight shift between button items, however, not with slot-item
it's work well.
Indeed, I've noticed it too. This is because of gap-y-2
on the wrapper. I've tried with space-y-2
or adding margin directly to the buttons but it does the same. I'll create an issue to keep track of this and merge this pr.
Indeed, I've noticed it too. This is because of
gap-y-2
on the wrapper. I've tried withspace-y-2
or adding margin directly to the buttons but it does the same. I'll create an issue to keep track of this and merge this pr.
I tried to remove gap-y-2
and adding mb-1
to button class and i think it's work well ?
Sorry guys but due to internet problems I couldn't work on the pull request the last few days. I had some comments/corrections but I see that the merge is already done. When I have a stable network, I'll just do additional PR in terms of performance or drafts with the thoughts that came to my mind.
Thanks :)
Resolves #362 Resolves #356
This PR adds to the
Accordion
component an additional mode of operation where you can customize the closing of panels when a specific list item is opened or global closing of panels whenever 1 list item is open.