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.

Sign up for GitHub

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

Jump to bottom

Migrate FeedbackWidget to shadcn #13456

Merged
merged 23 commits into from
Jul 26, 2024
Merged

Migrate FeedbackWidget to shadcn #13456

merged 23 commits into from
Jul 26, 2024

Conversation

wackerow
Copy link
Member

@wackerow wackerow commented Jul 23, 2024 •
edited
Loading

Description

  • Migrates FeedbackWidget and associated components to shadcn, deprecating use of Chakra dependencies
  • Adds custom useDisclosure hook, replacing Chakra import
  • Adds shadcn base popover component via command line

Preview URL

https://deploy-preview-13456--ethereumorg.netlify.app/en/
https://deploy-preview-13456--ethereumorg.netlify.app/nft/

Related Issue

shadcn migration

@github-actions github-actions bot added dependencies 📦 Changes related to project dependencies tooling 🔧 Changes related to tooling of the project labels Jul 23, 2024
@netlify Netlify
Copy link

netlify bot commented Jul 23, 2024 •
edited
Loading

✅ Deploy Preview for ethereumorg ready!

Name Link
🔨 Latest commit ca5da4c
🔍 Latest deploy log https://app.netlify.com/sites/ethereumorg/deploys/66a35f4eee659a000807474f
😎 Deploy Preview https://deploy-preview-13456--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: 46 (🔴 down 5 from production)
Accessibility: 93 (no change from production)
Best Practices: 91 (🔴 down 8 from production)
SEO: 92 (no change from production)
PWA: -
View the detailed breakdown and full score reports

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

@wackerow wackerow changed the title Shad feedback widget Migrate FeedbackWidget to shadcn Jul 23, 2024
@wackerow wackerow added the refactor ♻️ Changes which don't affect functionality label Jul 23, 2024
@wackerow wackerow mentioned this pull request Jul 23, 2024
Closed
@wackerow wackerow marked this pull request as ready for review July 23, 2024 22:41
@wackerow
deprecate: FeedbackGlyphIcon component
0dbfc89 
in lieu of .svg replacement; removed from Icon story
@konopkja
Copy link
Contributor

Screenshot 2024-07-24 at 20 18 23

think the secondary background color isnt necessary and could be removed

@konopkja
Copy link
Contributor

Screenshot 2024-07-24 at 20 18 23 think the secondary background color isnt necessary and could be removed

the dark brown

pettinarip
pettinarip requested changes Jul 25, 2024
View reviewed changes
Copy link
Member

@pettinarip pettinarip left a comment

Choose a reason for hiding this comment

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

Looks good @wackerow

One minor that I see on the preview is that the popover content moves a little bit when you hover the button and it scales.

bug.mp4

We could fix this by disabling the scaling effect when the popover is opened. What do you think?

src/components/FeedbackWidget/FixedDot.tsx Outdated Show resolved Hide resolved
src/components/FeedbackWidget/index.tsx Outdated Show resolved Hide resolved
wackerow and others added 4 commits July 25, 2024 12:53
@wackerow @pettinarip
fix: incorrect class and order
253e98e 
Co-authored-by: Pablo Pettinari <[email protected]>
@wackerow @pettinarip
chore: use doublequote
7c63636 
Co-authored-by: Pablo Pettinari <[email protected]>
@wackerow
fix: hover scale conditional
8eea331 
avoids popover from shifting while open
@wackerow
fix: remove hover shadow
0869c61
@wackerow
Copy link
Member Author

@konopkja Removed that shadow effect seen on hover

@pettinarip Conditionally paused the scale effect when open

Ready for re-review.

pettinarip
pettinarip approved these changes Jul 26, 2024
View reviewed changes
Copy link
Member

@pettinarip pettinarip left a comment

Choose a reason for hiding this comment

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

LGTM @wackerow

I've just pushed a commit to regenerate the yarn.lock file since it was installing an old version of @netlify/plugin-nextjs.

@pettinarip pettinarip merged commit 8feec78 into dev Jul 26, 2024
8 of 10 checks passed
@pettinarip pettinarip deleted the shad-feedback-widget branch July 26, 2024 11:42
This was referenced Aug 7, 2024
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pettinarip pettinarip pettinarip approved these changes

@corwintines corwintines Awaiting requested review from corwintines corwintines is a code owner

@minimalsm minimalsm Awaiting requested review from minimalsm minimalsm is a code owner

Assignees
No one assigned
Labels
dependencies 📦 Changes related to project dependencies refactor ♻️ Changes which don't affect functionality tooling 🔧 Changes related to tooling of the project
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@wackerow @konopkja @pettinarip