Skip to content

Conversation

@MOSHKA-GOT
Copy link
Contributor

Description

This PR updates the modal component to improve UX by adding open and close animations for both the modal content and the overlay background.

  • CodeExamples.tsx:

    • Removed conditional rendering of CodeModal to allow close animation before unmounting.
  • dialog-modal.tsx:

    • Replaced animate-contentShow with slide-up and slide-down animations.
    • Added overlay-fade-out animation for smooth background disappearance.
    • Adjusted overlay positioning (grid place-items-end, overflow-hidden).

These changes ensure that when the modal closes, it slides down instead of disappearing abruptly.

Before / After

▶ Before (open/close without smooth close)

1.mp4

▶ After (slide-up on open, slide-down on close, overlay fade)

2.mp4

@netlify
Copy link

netlify bot commented Aug 15, 2025

Deploy Preview for ethereumorg ready!

Name Link
🔨 Latest commit a125d87
🔍 Latest deploy log https://app.netlify.com/projects/ethereumorg/deploys/690035d49e8a400008ba7e16
😎 Deploy Preview https://deploy-preview-16074--ethereumorg.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
7 paths audited
Performance: 43 (🔴 down 7 from production)
Accessibility: 94 (no change from production)
Best Practices: 92 (🔴 down 8 from production)
SEO: 92 (no change from production)
PWA: 59 (no change from production)
View the detailed breakdown and full score reports

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

@MOSHKA-GOT
Copy link
Contributor Author

@chuyeow @matthieu @shazow @fjl Hey team, mind giving this PR a review?

@MOSHKA-GOT
Copy link
Contributor Author

@chuyeow @matthieu @shazow @fjl Hello team, would you be able to review this PR?

@wackerow
Copy link
Member

@pettinarip Mind giving this a review from a performance stand point?

@github-actions
Copy link
Contributor

This issue is stale because it has been open 30 days with no activity.

@github-actions github-actions bot added the Status: Stale This issue is stale because it has been open 30 days with no activity. label Oct 12, 2025
Copy link
Member

@wackerow wackerow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @MOSHKA-GOT! Sorry for delays on this one... Love the overall patch, but wanted to try and clean this up a bit to avoid adding unnecessary keyframes to the tailwind config.

Took a look, and this code modal behaves identical to the ui/sheet component we use, so I refactored CodeModal.tsx to use the sheet components instead of the dialogs. This allows simply applying side="bottom" for the animation.

Your approach to removing the isModalOpen && conditional indeed helps maintain the exit transition, and then radix-ui removes these nodes from the DOM tree when it's complete, which lgtm from a performance stand point.

Since the animations were simplified using ui/sheet, I reverted the tailwind config changes, leaving the animations as they were. While in there, patched the ui/dialog-modal component to use animate-fade-in which already existed, to avoid needing the other animate-contentShow or animate-overlayShow transitions.

As a small design aside, while in here I updated the close button to use "Close" which is in line with our more modern approach to these, added rounded corners to the top to match homepage design, and removed the excessive padding around the code block itself to clean things up.

Bringing this in! Thanks again for the help @MOSHKA-GOT!

@wackerow wackerow merged commit e8dd876 into ethereum:dev Oct 28, 2025
6 of 7 checks passed
@wackerow
Copy link
Member

@all-contributors please add @MOSHKA-GOT for code

@allcontributors
Copy link
Contributor

@wackerow

I've put up a pull request to add @MOSHKA-GOT! 🎉

This was referenced Nov 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Status: Stale This issue is stale because it has been open 30 days with no activity.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants