-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
chore: Simpler Sharing Page [Concept] #2361
chore: Simpler Sharing Page [Concept] #2361
Conversation
@gupta-piyush19 is attempting to deploy a commit to the formbricks Team on Vercel. A member of the Team first needs to authorize it. |
Thank you for following the naming conventions for pull request titles! 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @gupta-piyush19 Thanks for the PR, looks good and works well 😊
I've just added a small check to the useMembershipRole
hook to not call the api on the sharing page, other than that, LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gupta-piyush19 Looks good :-) I only have one concern regarding the modification of the generic useMembershipRole
hook:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gupta-piyush19 thanks for the changes; looks good :-)
Please go ahead with the merge :-)
…piyush/for-1977-concept-simpler-sharing-page
What does this PR do?
Simplifies the sharing page by using common component of analysis pages.
Fixes
https://linear.app/formbricks/issue/FOR-1977/[concept]-simpler-sharing-page
How should this be tested?
Checklist
Required
pnpm build
console.logs
git pull origin main
Appreciated