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

feat: recall from hidden fields and attributes #2601

Open
wants to merge 23 commits into
base: main
Choose a base branch
from

Conversation

Dhruwang
Copy link
Contributor

@Dhruwang Dhruwang commented May 10, 2024

What does this PR do?

Fixes #2353 #2327

  • Now we also allow recalling from hidden fields and attributes
  • Initially it was mainly focused on question, so we used namings like recallQuestion, selectRecallQuestion but now its more generic, so now we use recallItem, selectRecallItems
  • Breaks single response card into smaller components

Attributes

Screenshot 2024-05-10 at 12 02 34 PM

Hidden Fields

Screenshot 2024-05-10 at 12 01 36 PM

Also made the recall object more expandable to support other fields in future

How should this be tested?

For link surveys -> recall from questions and hidden fields is allowed
Fo app surveys -> recall from questions and attributes is allowed

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 May 10, 2024

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

2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
formbricks-cloud ⬜️ Ignored (Inspect) Visit Preview May 24, 2024 5:29am
formbricks-docs ⬜️ Ignored (Inspect) Visit Preview May 24, 2024 5:29am

Copy link
Contributor

github-actions bot commented May 10, 2024

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

@github-actions github-actions bot added the enhancement New feature or request label May 10, 2024
@jobenjada
Copy link
Member

hey @Dhruwang

thanks a lot, love how you always crank out a bunch of cool improvements out on a given day :D 🚀

Here's what I noticed:

  1. If the @ is the first character in the input, I cannot select the option:
image
  1. We need to make clear that the second input field is the placeholder for the second recall item:
image

Easiest would be to add a placeholder text with the recall item label so the hiddenField id or the question.


  1. The recall of attributes does not seem to work for me. I've recorded a video to show how I tried:
bug_attribute-not-working.mp4

  1. Kombobox / Search bar for dropdown: some users have thousands of attributes. It would make sense to add the same combobox that we're now using in the Logic Editor:

see here: #2584


Did you try your implementation with each question types? I normally find bugs when I do that 😉


Other than that, it works great :) Thanks!

@Dhruwang
Copy link
Contributor Author

Hey @jobenjada , have updated the PR ! 😊

Fallback input with placeholders

Screenshot 2024-05-13 at 12 52 18 PM

Searchable dropdown

Screenshot 2024-05-13 at 12 21 25 PM

Also tested the recall from attributes and it seems to work fine 🤔

Screen-Recording-2024-05-13-at-12.27.47.PM.mp4

In the video that you provided, did you miss the survey saving ?

@mattinannt mattinannt requested a review from jobenjada May 13, 2024 07:45
@jobenjada
Copy link
Member

Hey Dru!

Looks good :) Here's my feedback:

  1. Keyboard usability

With the search bar, we lost keyboard usability:
image

Pls make sure that Arrow down lets people select one of the fields. Similarly, if they go back up with the keys they should hop back into the Search bar once they reach the top of the list.


  1. UI changes in response card

As laid out in Slack


Please ping me once you've reworked the attribute logic, then I'll test it.

Thanks!

@jobenjada
Copy link
Member

looks and works great :) @mattinannt over to you 🤸

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 great! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
2 participants