ItsJonQ / g2

✨ An experimental reimagining of WordPress components
http://g2-components.com/
MIT License
105 stars 12 forks source link

Panel: Fix visible prop handling #253

Closed ItsJonQ closed 3 years ago

ItsJonQ commented 3 years ago

This update fixes the visible prop rendering for Panels OUTSIDE of Accordions.

There was a bug in the useAccordion hook that prevent the visible prop from being correctly evaluated without rendering within an Accordion Context.

vercel[bot] commented 3 years ago

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/itsjonq/g2/r339d2pfb
✅ Preview: https://g2-git-fix-panel-initial-visible.itsjonq.vercel.app

sarayourfriend commented 3 years ago

@ItsJonQ What's the best way to test this?

ItsJonQ commented 3 years ago

@saramarcondes Ah! My apologies 🙏 .

Make sure these Panels (Accordions) still sync with each other (when open/closing) https://g2-git-fix-panel-initial-visible.itsjonq.vercel.app/iframe.html?id=components-accordion--default&viewMode=story

Ensure this Panel is initially expanded (and can be closed/opened) https://g2-git-fix-panel-initial-visible.itsjonq.vercel.app/iframe.html?id=components-panel--default&viewMode=story

sarayourfriend commented 3 years ago

@ItsJonQ So it appears that the accordions only sync when they're opening, but they do not close together. Is that the intended behavior?

ItsJonQ commented 3 years ago

but they do not close together. Is that the intended behavior?

🤦 no, haha. They should be closing together as well.

I think a good order would be to merge your https://github.com/ItsJonQ/g2/pull/255 update, and continue with this PR.

ItsJonQ commented 3 years ago

@saramarcondes Investigated! It looks like it's an existing issue. I've added a todo for a fix. We can do it in a follow up :)