Add support for additional survey question type "open_text"

[felixarntz]

When the initial user feedback survey was introduced, scope of the MVP was limited to only support questions of the "rating" type, since that was the only one used for the initial survey. This should now be followed up on, with this issue focusing on the "open_text" question type.

---

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

• A new SurveyQuestionOpenText JS component should be introduced and integrated into the user feedback survey JS infrastructure, rendering a survey question of type "open_text".
• The UI should look like the above screenshot and follow the "rating" question type where similar (e.g. font, spacing, G icon etc.).
• The available arguments to consider here from the Site Kit Service /survey/trigger/ API response are as follows:
  • question.question.subtitle should, if present, be displayed right below the text input (see screenshot).
  • question.question.placeholder should, if present, be displayed as a text input placeholder (see screenshot).
  • The text input must be non-empty in order to proceed. The maximum character limit should be 100.
• The data object for answering the question (what needs to be passed to the answerQuestion function prop) should look as follows:
  • A single answer property with the string from the text input.
  • Example: { answer: 'My open text response' }
• The new question type should have an example in Storybook where it can be inspected, similar to the existing "rating" type component.

Implementation Brief

Create new component

Create assets/js/components/surveys/SurveyQuestionOpenText.js

Base on SurveyQuestionRating.js

• wrap everything in a div with class .googlesitekit-survey__open-text
• Use SurveyHeader to match styling
• Use the TextField, Input, HelperText components
• Add the following string props and propTypes: question, subtitle, placeholder
• Add the following function props and propTypes: answerQuestion, dismissSurvey
• HelperText should be passed into TextField's helperText prop (with the child subtitle)
• In order to limit user input to 100 characters, just disallow extra characters in the TextField's onChange handler (slicing the text to 100 characters)
• If the TextField is empty, set a disabled prop to true on the submit button
• When submitting, pass the answer to answerQuestion as per AC

In assets/sass/components/surveys/_googlesitekit-surveys.scss

• Add a new rule for .googlesitekit-survey__open-text
• Nest rules for .googlesitekit-survey__body, .mdc-text-field, and .mdc-text-field-helper-line
• Style now to match the design

Add new component to map on `assets/js/components/surveys/CurrentSurvey.js§

• ComponentMap also needs to map open_text to SurveyQuestionOpenText

❗ ❗ See PR here with everything done up until here #3771 ❗ ❗

Create new story

Create assets/js/components/surveys/SurveyQuestionOpenText.stories.js (Based on SurveyQuestionRating.stories.js)

• Create story showing SurveyQuestionOpenText with props and mock handlers to show it working

Test Coverage

• Add new tests within assets/js/components/surveys/CurrentSurvey.test.js for the new question type.

We need to add this question type to the "main" test suite for CurrentSurvey to make sure the question type works as expected when "plugged in" to the rest of the survey components, next button, etc.

Visual Regression Changes

• No changes

QA Brief

• Check that https://google.github.io/site-kit-wp/storybook/develop/?path=/story/components-surveys--survey-question-open-text-story works and behaves as per AC
• Enter the code from https://pastebin.com/x0iiFBWh into your browser console on a Site Kit dashboard page to trigger a survey with a question from Add support for additional survey question type "single_select" #3760, Add support for additional survey question type "multi_select" #3761, and this issue (Add support for additional survey question type "open_text" #3762).

Changelog entry

• Add an open text type to user surveys.

[danielgent]

This is what I've done with minimal changes to our existing components (looks similar to the design but definitely not identical)

Here you can see it working

https://google.github.io/site-kit-wp/storybook/pull/3771/?path=/story/components-surveys--survey-question-open-text-story

[eugene-manuilov]

@danielgent, please, add IB.

[danielgent]

I've had a go @eugene-manuilov. 🤷

Seems less clear to me than just reading the actual code, so I'm not sure if I've done what's expected

[eugene-manuilov]

Thanks, @danielgent. It's better than just "merge my PR", but still requires a bit more infomration. Could you please add more details? When you write IB, think that you write it for someone who is not familiar with your PR, so once they read your IB they understand what needs to be done. When I read an instruction like Use SurveyHeader to match styling, it doesn't help me at all, it just adds more questions: which styles?, how should i use it?, where should i use it?, etc. The same applies to other instructions.

One more thing that is missing in IB is how the new question component should be added to the CurrentSurvey component. Which modifications need to be done there and in the SurveyQuestionRating component.

[danielgent]

@eugene-manuilov I don't see the AC telling me to touch the CurrentSurvey component, especially if the SurveyQuestionRating requires modifying as we have a few similar new components

I'm going to put changes to this IB on hold for now. The whole diff of the working PR is only about 100 actual lines of code, of that most, is copy-pasted.

[eugene-manuilov]

I don't see the AC telling me to touch the CurrentSurvey component, especially if the SurveyQuestionRating requires modifying as we have a few similar new components

@danielgent it assumes to do it. The Add support for additional survey question type ... title means that that additional question type should be supported by the survey infrastructure, correct? We don't have a separate ticket that will ask us to update the CurrentSurvey component to use the new SurveyQuestionOpenText component.

[aaemnnosttv]

Thanks @danielgent – you're right that the ACs don't mention changing the CurrentSurvey component, but @eugene-manuilov is also right that there is a very simple change needed there in the ComponentMap (outside of the component) which maps each question type to the component type that doesn't make sense to do in a separate issue, which I see you've done in your PR 👍

I think the IB is mostly good to go here with one important caveat and that is the absence of test coverage which is often not explicitly called for in the ACs. As mentioned on #3760 (comment) we want to make sure each question type is added to the "main" test suite for surveys which are the tests for CurrentSurvey to make sure the question type works as expected particularly when "plugged in" to the rest of the survey components, next button, etc. This is the important part to add. Optionally, more tests could be added for the component itself in a separate file for the component as we normally would but perhaps that's not really needed in addition to the others for such a simple input here. This will of course increase the estimate a bit but that's to be expected.

[aaemnnosttv]

IB ✅

[danielgent]

I've added a test for this question type to CurrentSurvey.test.js

This wasn't mentioned in the IB though so I don't think it's been thought through 🤔

In order for the test to pass, it requires changes to the CurrentSurvey component, as SurveyQuestionOpenText requires different props, and CurrentSurvey needs to pass these in.

I'll just make these changes in anyway and then after we can discuss the best way

[danielgent]

Assigning back to you @tofumatt for another round of CR (I have some questions)

I also did 3761 and 3760 and they're very similar so

1. I'm going to assign them to you (hope you don't mind)
2. I'm going to do similar changes to the tests (break them up, turn the comments into full sentences) 👍

[wpdarren]

@danielgent sure, I will take another look at this.

[wpdarren]

QA Update: ❌

@felixarntz @tofumatt since Daniel has departed, I still have two observations that I would like to raise.

1. On the Storybook within the QAB, the padding between the input field and submit button seems too big. I have cleared cache and viewing it in Chrome. This is on desktop and mobile. When I also used the pastebin code on my test site, the same padding issue occurs, so think we need to look at the styling What are your thoughts?

2. The placeholder text in the input field is different to Storybook. Is that just the pastebin code that's causing that? I am assuming it is but would like to check.

[aaemnnosttv]

Currently outputting an empty helper text div when there is no text.

[aaemnnosttv]

Ready for another pass @wpdarren 👍

[wpdarren]

QA Update: ✅

Verified

• Verified that the open text form appears and is displayed as per AC.
• Checked that Storybook works and behaves as per AC

[felixarntz]

Approval ❌

Similar to #3761 (comment), the implementation here does not match the ACs, as it is relying on question.subtitle and question.placeholder instead of question.question.subtitle and question.question.placeholder.

The overall question schema is always only including the question_ordinal, question_text, question_type and question fields. Any question type-specific fields are always nested within the last field, i.e. in question.question.

cc @tofumatt @aaemnnosttv