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

Add useOnClickOutside hook #90

Merged
merged 5 commits into from
Apr 7, 2024
Merged

Add useOnClickOutside hook #90

merged 5 commits into from
Apr 7, 2024

Conversation

jarrisondev
Copy link
Member

@jarrisondev jarrisondev commented Apr 6, 2024

Changes Made 🎉

  • feat: added new feature to improve user experience
  • fix: corrected a bug with login functionality
  • refactor: improved code readability and organization
  • docs: updated README with new instructions
  • chore: updated dependencies and configuration files

Describe Changes

  • Create useOnClickOutside hook to handle the click outside an element
  • Implement the hook in the Avatar component
  • Add responsive design in navbar with loggedUser

Related Issue(s)

#86

Visuals (Optional)

image

Checklist ✅

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

Copy link

vercel bot commented Apr 6, 2024

@jarrisondev is attempting to deploy a commit to the aforina's projects Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Member

@Zyruks Zyruks left a comment

Choose a reason for hiding this comment

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

Minor improvements, GREAT PR!

@Zyruks Zyruks added the Type: Enhancement Suggest an improvement for an existing feature. label Apr 6, 2024
@jarrisondev
Copy link
Member Author

@Zyruks Do not merge, I'm working on this issue #86

Copy link
Contributor

@MarcosNASA MarcosNASA left a comment

Choose a reason for hiding this comment

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

Looks mergeable to me too, nice job! 🌮 🌯

type Event = MouseEvent | TouchEvent;

export const useOnClickOutside = (ref: RefObject<HTMLElement>, handler: (event: Event) => void) => {
useEffect(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

🟢 I'd suggest using the latest ref pattern to avoid forcing the consumer of the hook to memoize the handler function to avoid the event from being bound and unbound on every rerender.
You can see an example here,

@afordigital
Copy link
Member

Very good PR Jarri! Amazing!! Thanks a lot for the contribution 😁!

@afordigital afordigital merged commit 81cdae3 into Afordin:main Apr 7, 2024
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement Suggest an improvement for an existing feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants