Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#267 Fix Checkbox in Dialog #268

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

#267 Fix Checkbox in Dialog #268

wants to merge 2 commits into from

Conversation

amcdnl
Copy link
Member

@amcdnl amcdnl commented Dec 3, 2024

This PR:

  • Fixes the checkbox not having the correct path animation set when in a dialog/drawer/et.
  • Adds storybook case for testing
  • Fixes setTimeout type broken

@amcdnl amcdnl added the bug Something isn't working label Dec 3, 2024
@amcdnl amcdnl requested a review from steppy452 December 3, 2024 12:39
@amcdnl amcdnl self-assigned this Dec 3, 2024
Copy link

netlify bot commented Dec 3, 2024

Deploy Preview for reablocks-storybook ready!

Name Link
🔨 Latest commit 05eca38
🔍 Latest deploy log https://app.netlify.com/sites/reablocks-storybook/deploys/674efc8cc32eda0008bf4efb
😎 Deploy Preview https://deploy-preview-268--reablocks-storybook.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@steppy452
Copy link
Contributor

@amcdnl hrmm not seeing that fix the issue

Screen.Recording.2024-12-03.at.4.10.32.PM.mov

@steppy452
Copy link
Contributor

steppy452 commented Dec 4, 2024

@amcdnl I was playing around with this a little bit and got it to work by forcing a re-render after initial mount by updating state:

// If the checkbox is inside a dialog, the animation will not work.
// This is a workaround to force the animation to work by triggering
// a re-render once after initial mount
const [_, setForceAnimation] = useState<boolean>(false);
useEffect(() => {
  if (checked || intermediate) {
    setForceAnimation(true);
  }
}, []); // eslint-disable-line react-hooks/exhaustive-deps

It's definitely a little hacky, but wondering what you think about this.

Screen.Recording.2024-12-03.at.7.49.17.PM.mov

@steppy452 steppy452 requested a review from evgenoid December 4, 2024 01:54
@amcdnl
Copy link
Member Author

amcdnl commented Dec 19, 2024

@steppy452 - Maybe its a timing thing/race condition w/ my fix. I swear it worked lol.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants