Skip to content

Conversation

@anjy7
Copy link
Contributor

@anjy7 anjy7 commented Oct 30, 2023

What does this PR do?

Fixes #1444

Loom Video: https://www.loom.com/share/3650caac8e4c43c098a645b309b10665?sid=85821dd8-7725-4140-8a18-47093197425a

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (refactoring code, technical debt, workflow improvements)
  • Enhancement (small improvements)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change adds a new database migration
  • This change requires a documentation update

How should this be tested?

  • Test A
  • Test B

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

@vercel
Copy link

vercel bot commented Oct 30, 2023

@anjy7 is attempting to deploy a commit to the formbricks Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions github-actions bot added enhancement New feature or request formtribe-2023 hacktoberfest complete these issues to gather points for Hacktoberfest ❗️ migrations labels Oct 30, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Oct 30, 2023

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

@anjy7 anjy7 changed the title feat: feature to add image/color/animation as survey background feat: add image/color/animation as survey background Oct 30, 2023
@neilchauhan2
Copy link
Contributor

Hi @anjy7, thank you for your contribution. I tested this feature on local and faced two issues,

  • The width of the survey input is smaller than what we usually have, you can go to app.formbricks.com and create a survey to refer the actual width.
  • Once we try to manipulate the height of the input field, the UI breaks.
    Please refer the screen recording shared below:
Screen.Recording.2023-10-31.at.2.44.38.PM.mov

cc: @mattinannt

@anjy7
Copy link
Contributor Author

anjy7 commented Oct 31, 2023

Hi @anjy7, thank you for your contribution. I tested this feature on local and faced two issues,

* The width of the survey input is smaller than what we usually have, you can go to app.formbricks.com and create a survey to refer the actual width.

* Once we try to manipulate the height of the input field, the UI breaks.
  Please refer the screen recording shared below:

Screen.Recording.2023-10-31.at.2.44.38.PM.mov

cc: @mattinannt
Ahh I see, thanks..I'll fix it :)

@anjy7
Copy link
Contributor Author

anjy7 commented Oct 31, 2023

@anjy7 anjy7 marked this pull request as ready for review October 31, 2023 15:25
@jobenjada
Copy link
Member

@neilchauhan2 passing it back to you :)

I've solved most of the UI / UX issues, except for one: content position of images:

image

Task 1: Center the background image in the frame vertically and horizontally..

Code Review

We have quite some redundant code in:

  • SurveyBg.tsx
  • PreviewSurveyBgDeskstop.tsx (incl. typo :)

Task 2: Please refactor so that it is reusable and follows the DRY principle.

Task 3: Apart from that, please go through the complete PR and see if you notice anything else which deviates from our How we code at Formbricks best practices.

Task 4: Make a complete end-to-end test of the UI & UX. Catch all the bugs / inconsistencies which could arise with other link survey functionality.

Thanks 🙏

@mattinannt mattinannt assigned jobenjada and unassigned neilchauhan2 Nov 27, 2023
@jobenjada
Copy link
Member

jobenjada commented Nov 27, 2023

whats missing is to refactor MediaBackground so that we can remove the redundant code in PreviewSurveyBgMobile.tsx

Merge conflicts

@jobenjada
Copy link
Member

@mattinannt allllllrighty, seems all good! It works well, just getting a build error but dont think its related to my refactoring:

image

Pls resolve and merge 🚀

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.

LGTM

@mattinannt
Copy link
Member

Just updated the data model and added a more generic styling property to the survey model in preparation for the upcoming improved styling functionality.
Ready to merge!!!!!! Great job everyone! 🚀

@anjy7
Copy link
Contributor Author

anjy7 commented Dec 4, 2023

Lets gooooo!!💪🏻🔥

@mattinannt mattinannt enabled auto-merge December 4, 2023 18:19
@mattinannt mattinannt added this pull request to the merge queue Dec 4, 2023
Merged via the queue into formbricks:main with commit 9271e37 Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request hacktoberfest complete these issues to gather points for Hacktoberfest ❗️ migrations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE][ LAST FormTribe 🔥][1000 Points] Multi Media Backgrounds

4 participants