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

feature: paragraph ItemResponseType for phrase builder response options WEB (M2-7762) #542

Merged

Conversation

AlejandroCoronadoN
Copy link
Contributor

@AlejandroCoronadoN AlejandroCoronadoN commented Oct 24, 2024

  • Added paragraphText phrasalTemplateCompatibleResponseTypes
  • Assign item.reponseType with value paragraphText to the text-type rendering formatting on the ActionPlan. The entire paragraph should apper between fields of the ActionPlan.

📝 Description

As an Admin, I want to choose the paragraph item type as a response phrase field so that I can include its contents in my Action Plan. By adding paragraph option to the allowed types on validate_phrasal_templates, the backend can store Activities with PhrasalBuilder Items that contain Parragraph items as elements of the PhraseBuilder item.

🔗 Jira Ticket M2-7762

Changes include:

  • The entire paragraph should apper between fields of the ActionPlan.

📸 Screenshots

image

🪤 Peer Testing

  • Create an activity
  • Create and item of type parragraph inside your activity
  • In the same activity create a new item of type phrase builder
  • Phrase builder should allow you to choose the newly created paragraph item inside the “Select Item (Response)” dropdown menu.
  • Complete an activity from the web application and validate that the parragraph can be selected on the Phrase Builder section.
  • Invite a user to the new applet/activity
  • Logged as a user complete the new applet/activity
  • On the last section of the Activity you will be able to see the new Action Plan with the paragraph inserted between the fields of the PhraseBuilder-ActionPlan-PhrasalTemplate

Copy link

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-542.d15zn9do8xbzga.amplifyapp.com

Copy link
Contributor

@farmerpaul farmerpaul left a comment

Choose a reason for hiding this comment

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

Hey @AlejandroCoronadoN, I found that one of the bullets in AC wasn't satisfied:

  • An action plan configured with a phrase containing Paragraph Text item types will display the paragraph text, including line breaks / new lines

When I provided a paragraph of text with newlines in it, the newlines were removed in the Action Plan:

Paragraph input Action Plan
CleanShot 2024-10-29 at 16 30 44@2x CleanShot 2024-10-29 at 16 30 54@2x

@AlejandroCoronadoN AlejandroCoronadoN marked this pull request as draft October 29, 2024 20:02
@farmerpaul farmerpaul self-requested a review October 29, 2024 20:07
@AlejandroCoronadoN AlejandroCoronadoN marked this pull request as ready for review October 30, 2024 21:41
@AlejandroCoronadoN
Copy link
Contributor Author

Hello @farmerpaul . Here are more details about my solution:

  1. I added the conditional logic for paragraph type on fieldPhrasalDataType === paragraph
  2. I splited my text into multiple "words" using breaklines so each "word" that is an individual sentence will be rendered in a separate
  3. I added the conditional rendering for fieldPhrasalDataType === 'paragraph' inside the Text component. Here I rendered all the individual sentences in a different span. That way we can se a breakline between each paragraph.

image
So this renders like this:
image

Following up with my comments on slack, I think there is another approach that involves rendering conditional on the fieldDisplayMode value. For this implementation I will need to:

  1. Add a new fieldDisplayMode on the Admin application.
  2. When creating the items I will need to insert the new displayMode for paragraphs
  3. Using the NEW fieldDisplayMode I will render paragraphs differently. So the second condition where I use fieldPhrasalDataType === 'paragraph' will be replaced by fieldDisplayMode === 'paragraph'. Making the code cleaner.

Copy link
Contributor

@farmerpaul farmerpaul left a comment

Choose a reason for hiding this comment

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

Hey @AlejandroCoronadoN, thanks for making that change, it's almost there! I just noticed a couple ways the code could be made a bit clearer/simpler, and also wanted to draw your attention to a detail about consecutive line breaks.

Also, thank you for introducing me to flatMap()! I can't believe I've never really encountered that before… 🙈🙈🙈

Copy link
Contributor

@farmerpaul farmerpaul 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, thank you for making those changes! Just made one very minor type suggestion, and identified a rogue eslint-disable line that I wasn't sure was intentional. Anyway, PR is ready for QA 🙂 🙌🏻

@AlejandroCoronadoN AlejandroCoronadoN merged commit e6fb19e into dev Nov 12, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants