-
Notifications
You must be signed in to change notification settings - Fork 902
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: Add welcome card #1073
feat: Add welcome card #1073
Conversation
@KapadiaNaitik 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! 🙏 |
Hey @KapadiaNaitik how is it? Is this ready for review as discussed on Discord? :) Would love to release the other upload related issues today which would reuse some of your code, so this should get merged today :pray |
1 similar comment
Hey @KapadiaNaitik how is it? Is this ready for review as discussed on Discord? :) Would love to release the other upload related issues today which would reuse some of your code, so this should get merged today :pray |
Hey, @jobenjada it's done except for the Lucide Icon for enter and I was facing some issues with types/react and Lucide icon. I have already talked to and informed @pandeymangg and he had a look(it was a weird dependency issue afaik). I will follow up once again with him and get this done as soon as possible. Sorry for the delay |
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.
Thanks a lot @KapadiaNaitik for this PR! looks and works great! 🥳
I have left some comments, please go through them. For using the Image component from next/image
, you'll need to modify the images.remotePatterns
field in the next.config.mjs
file under apps/web
. Please add the following at the end of that array:
{
protocol: "http",
hostname: "localhost",
},
{
protocol: "https",
hostname: "app.formbricks.com",
},
...[environmentId]/surveys/[surveyId]/(analysis)/summary/components/shareEmbedTabs/EmailTab.tsx
Outdated
Show resolved
Hide resolved
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/edit/WelcomeQuestionForm.tsx
Outdated
Show resolved
Hide resolved
return ( | ||
<div> | ||
{question.companyLogo && ( | ||
<img |
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.
Please use the Image component from next/image
here 🙏
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.
next/Image not working for some reason
@jobenjada I have pushed the required tweaks |
LGTM! 🥳 |
Hey @mattinannt looks like I and you both pushed our commits so welcomeCard got duplicated in clientSettings😅 |
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
What does this PR do?
Fixes #1055
Type of change
How should this be tested?
Checklist
Required
pnpm build
console.logs
git pull origin main
Appreciated