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

chore: Link survey mobile view #2493

Merged
merged 12 commits into from
Apr 24, 2024
Merged

chore: Link survey mobile view #2493

merged 12 commits into from
Apr 24, 2024

Conversation

Dhruwang
Copy link
Contributor

What does this PR do?

Fixes https://github.com/formbricks/internal/issues/62

Screenshot 2024-04-19 at 2 33 53 PM Screenshot 2024-04-19 at 2 33 27 PM

It may break if you add too much content (long headline and description) in a question but it can be fixed with #2477

How should this be tested?

Test link survey on small devices

Checklist

Required

  • Filled out the "How to test" section in this PR
  • Read How we Code at Formbricks
  • Self-reviewed my own code
  • Commented on my code in hard-to-understand bits
  • Ran pnpm build
  • Checked for warnings, there are none
  • Removed all console.logs
  • Merged the latest changes from main onto my branch with git pull origin main
  • My changes don't cause any responsiveness issues
  • First PR at Formbricks? Please sign the CLA! Without it we wont be able to merge it 🙏

Appreciated

  • If a UI change was made: Added a screen recording or screenshots to this PR
  • Updated the Formbricks Docs if changes were necessary

Copy link

vercel bot commented Apr 19, 2024

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

Name Status Preview Comments Updated (UTC)
formbricks-cloud ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 24, 2024 0:52am
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
formbricks-com ⬜️ Ignored (Inspect) Visit Preview Apr 24, 2024 0:52am

Copy link
Contributor

github-actions bot commented Apr 19, 2024

Thank you for following the naming conventions for pull request titles! 🙏

@Dhruwang Dhruwang changed the title Link survey mobile view chore: Link survey mobile view Apr 19, 2024
@jobenjada
Copy link
Member

Hey Dhru!

Thanks, looks good. However, if I test it on my iPhone, I have to scroll up to see the survey:

IMG_2436
IMG_2437

Other than that, I like the filling out UX much better. Really easy within the thumb range :)

Copy link
Member

@jobenjada jobenjada left a comment

Choose a reason for hiding this comment

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

see comment

@Dhruwang
Copy link
Contributor Author

Hey @jobenjada, Updated the PR 😊. For safari on mobile, there is an issue with the dynamic height of address bar, and due to which even though its 100vh, it still scrolls because address bar at the bottom takes some height

@vercel vercel bot temporarily deployed to Preview – formbricks-cloud April 23, 2024 15:10 Inactive
Copy link
Member

@jobenjada jobenjada left a comment

Choose a reason for hiding this comment

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

Hey Dhru!

Noticed two more things:

The bottom border corners shouldn't be rounded on mobile:

image

Secondly, we currently have uneven paddings for top and bottom (pt-6 and pb-3) which is good for Desktop and in-app.

On mobile, pls sync them so its pt-6 and pb-6

So should look like this:

image

Thanks a lot! :)

@@ -40,5 +40,5 @@ export const SurveyInline = (props: Omit<SurveyInlineProps, "containerId">) => {
loadScript();
}, [containerId, props, renderInline]);

return <div id={containerId} className="h-full w-full" />;
Copy link
Member

Choose a reason for hiding this comment

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

did you test this well? Feels like it'd break the layout in other places ;)

@Dhruwang
Copy link
Contributor Author

Hey @jobenjada , have updated the PR 😊
Screenshot 2024-04-24 at 6 00 33 PM

Copy link
Member

@jobenjada jobenjada 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!

@jobenjada jobenjada added this pull request to the merge queue Apr 24, 2024
Merged via the queue into main with commit dcc98b6 Apr 24, 2024
12 checks passed
@jobenjada jobenjada deleted the link-survey-mobile-view branch April 24, 2024 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants