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

feat: migrate headless ui components to radix primitives #4517

Closed
wants to merge 15 commits into from
Closed

feat: migrate headless ui components to radix primitives #4517

wants to merge 15 commits into from

Conversation

sdthakral33
Copy link

@sdthakral33 sdthakral33 commented Jan 10, 2024

What does this PR do?

Related issues

/claim #4327
Fixes #4327

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Enhancement (non-breaking small changes to existing functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Explanation of the changes

All the components like menus, modals and tabs using headlessui components have been migrated to radix ui primitive components

Copy link

vercel bot commented Jan 10, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
web ❌ Failed (Inspect) Jan 20, 2024 7:35am

Copy link

vercel bot commented Jan 10, 2024

@sdthakral33 is attempting to deploy a commit to the Hey Team on Vercel.

A member of the Team first needs to authorize it.

@sdthakral33
Copy link
Author

@bigint Please review this PR, I have closed the previous PR because of its huge divergence from main branch. I have fixed all the issue reported by you in the previous PR.
Thanks!

@sdthakral33
Copy link
Author

@bigint Any Updates?

packages/ui/src/Modal.tsx Outdated Show resolved Hide resolved
apps/web/src/styles.css Outdated Show resolved Hide resolved
apps/web/src/styles.css Outdated Show resolved Hide resolved
apps/web/src/components/Shared/Sidebar/SidebarMenu.tsx Outdated Show resolved Hide resolved
apps/web/src/components/Shared/Navbar/WalletUser.tsx Outdated Show resolved Hide resolved
apps/web/src/components/Shared/Navbar/WalletUser.tsx Outdated Show resolved Hide resolved
@sdthakral33
Copy link
Author

@bigint Addressed all the review comments. Please review!

@sdthakral33
Copy link
Author

@bigint Any Updates on this?

@bigint
Copy link
Member

bigint commented Jan 15, 2024

Screen.Recording.2024-01-15.at.10.27.38.AM.mov

dropdown misplaced, @sdthakral33 can you please cross check with prod? we need it to be pixel perfect 🙏🏼

@bigint
Copy link
Member

bigint commented Jan 15, 2024

Screen.Recording.2024-01-15.at.10.28.55.AM.mov

cls because of scroll gets hidden on overall layout

@sdthakral33
Copy link
Author

@bigint Please check now!!!

@sdthakral33
Copy link
Author

@bigint Any Updates???

@bigint
Copy link
Member

bigint commented Jan 17, 2024

Screen.Recording.2024-01-17.at.10.10.26.AM.mov

opening modals are not smooth tbh

@sdthakral33
Copy link
Author

@bigint Please check now, added transition to Dialog Overlay also.

@bigint
Copy link
Member

bigint commented Jan 18, 2024

Screen.Recording.2024-01-18.at.8.20.25.AM.mov

I'm still seeing the same, also seeing white blink, you can watch this video in slow-mo!

@bigint
Copy link
Member

bigint commented Jan 18, 2024

I see lot of back and forth 🙇🏼, feel free to don't stress out 🤞🏼

@sdthakral33
Copy link
Author

I see lot of back and forth 🙇🏼, feel free to don't stress out 🤞🏼

@bigint - But the effect is the same that was present in headless UI code. And IMO, the behaviour also seems to same. Can you post a video to show a comparison panel between production and my branch

@bigint
Copy link
Member

bigint commented Feb 7, 2024

Closing this for now, as radix is not meeting Hey's requirements!

@bigint bigint closed this Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Migrate away from all headless ui usage to use radix primitives
2 participants