SelectPanel2: 3 tiny bug fixes #3770
Conversation
|
size-limit report 📦
|
siddharthkp
temporarily deployed
to
github-pages
GitHub Actions
Inactive
Just wanted to ask, is it possible for |
This reverts commit 02901a8.
I think so, i have a rough idea. But, I might be wrong. Will find out, I guess :) |
| if (typeof props.onCancel === 'function') props.onCancel() | ||
| } | ||
| // @ts-ignore todo | ||
| const onInternalSubmit = event => { | ||
| event.preventDefault() | ||
| if (props.open === 'undefined') setInternalOpen(false) | ||
| if (props.open === undefined) setInternalOpen(false) |
| @@ -105,7 +105,7 @@ const SelectPanel = props => { | |||
| {slots.header || <SelectPanel.Header />} | |||
| {childrenInBody} | |||
| {/* render default footer as fallback */} | |||
| {slots.footer || <SelectPanel.Footer />} | |||
| {slots.footer} | |||
There was a problem hiding this comment.
I forgot that we would need to add Suspense support to useSlots first before making footer optional.
This is probably my ignorance but what is the reason of wrapping the footer in Suspense? 🤔
There was a problem hiding this comment.
If we need to fetch data before rendering the list, we'd like the footer to also be hidden until then.
So it's wrapped in the same Suspense boundary as list
Screen.Recording.2023-09-28.at.4.01.41.PM.mov
Fix 1
In #3758, I forgot that we would need to add Suspense support to
useSlotsfirst before making footer optional. Default header is fine because it can't be wrapped in suspense, but footer totally can be.Haven't tried to update useSlots yet, will explore later.
Fix 2
The menu was not closing on it's own because I made a silly mistake. We will be able to catch these silly things once I write some tests 😅
Fix 3
Added a warning to the ActionMenu story because it's implementation will most definitely change once we implement SelectPanel as a modal