feat(surveys): opt-in back button for multi-question surveys#3634
Open
lucasheriques wants to merge 2 commits into
Open
feat(surveys): opt-in back button for multi-question surveys#3634lucasheriques wants to merge 2 commits into
lucasheriques wants to merge 2 commits into
Conversation
Adds an appearance-gated `allowGoBack` flag that renders a "Back" button on web surveys after the first question. Uses a visited-index history stack persisted in localStorage so navigation respects branching paths (response_based, specific_question) — going back returns you to the question you actually came from, not currentIndex-1. Prior answers are pre-filled on the question you return to. Generated-By: PostHog Code Task-Id: d229cf67-cbe1-43d1-9b8f-fa32ae50eca6
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Contributor
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+34
to
+66
| {(showBackButton || !skipSubmitButton) && ( | ||
| <div className={`form-buttons${showBackButton ? ' form-buttons-with-back' : ''}`}> | ||
| {showBackButton && ( | ||
| <button | ||
| className="form-back" | ||
| type="button" | ||
| aria-label="Go to previous question" | ||
| onClick={onBack} | ||
| > | ||
| {appearance.backButtonText || 'Back'} | ||
| </button> | ||
| )} | ||
| {!skipSubmitButton && ( | ||
| <button | ||
| className="form-submit" | ||
| disabled={submitDisabled} | ||
| aria-label="Submit survey" | ||
| type="button" | ||
| onClick={() => { | ||
| if (link) { | ||
| window?.open(link) | ||
| } | ||
| if (isPreviewMode) { | ||
| onPreviewSubmit?.() | ||
| } else { | ||
| onSubmit() | ||
| } | ||
| }} | ||
| > | ||
| {text} | ||
| </button> | ||
| )} | ||
| </div> |
Contributor
There was a problem hiding this comment.
form-buttons div
Even when allowGoBack is false (and showBackButton is therefore false), the condition (showBackButton || !skipSubmitButton) is true whenever the submit button is visible — so the submit button is now wrapped in <div class="form-buttons"> for every ordinary survey. This changes the DOM structure for all existing surveys, not just those opting into back navigation. Any host-page CSS that targets .bottom-section > button or .bottom-section > .form-submit would silently break because there is now an intermediate div between them. Consider restoring the original single-button layout when showBackButton is false.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/browser/src/extensions/surveys/components/BottomSection.tsx
Line: 34-66
Comment:
**Submit button always wrapped in new `form-buttons` div**
Even when `allowGoBack` is false (and `showBackButton` is therefore `false`), the condition `(showBackButton || !skipSubmitButton)` is `true` whenever the submit button is visible — so the submit button is now wrapped in `<div class="form-buttons">` for every ordinary survey. This changes the DOM structure for all existing surveys, not just those opting into back navigation. Any host-page CSS that targets `.bottom-section > button` or `.bottom-section > .form-submit` would silently break because there is now an intermediate `div` between them. Consider restoring the original single-button layout when `showBackButton` is false.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+101
to
+120
| fireEvent.click(backButton) | ||
|
|
||
| await waitFor(() => expect(screen.getByText('Question 1')).toBeVisible()) | ||
| expect(screen.getByRole('textbox')).toHaveValue('first answer') | ||
| expect(screen.queryByRole('button', { name: /go to previous question/i })).not.toBeInTheDocument() | ||
| }) | ||
|
|
||
| test('navigation history survives a re-render (resume from persisted state)', async () => { | ||
| // First mount: advance Q1 -> Q2, capture whatever state the SDK persisted. | ||
| const { unmount } = render( | ||
| <SurveyPopup survey={baseSurvey} removeSurveyFromFocus={jest.fn()} isPopup posthog={mockPosthog as any} /> | ||
| ) | ||
|
|
||
| fireEvent.input(screen.getByRole('textbox'), { target: { value: 'first answer' } }) | ||
| fireEvent.click(screen.getByRole('button', { name: /submit survey/i })) | ||
| await waitFor(() => expect(screen.getByText('Question 2')).toBeVisible()) | ||
|
|
||
| const persistedState = mockedSetInProgressSurveyState.mock.calls.at(-1)![1] | ||
| unmount() | ||
|
|
Contributor
There was a problem hiding this comment.
waitFor / async in test that checks async state
The test 'back button is hidden when allowGoBack is not set' is a synchronous test that asserts screen.getByText('Question 2') directly after fireEvent.click. Every other test in this file that checks post-click state uses await waitFor(...) and is async. If the Preact state update ever becomes deferred, this test will fail with an unhelpful "element not found" error instead of a meaningful assertion failure. The test should be async and use await waitFor(() => expect(screen.getByText('Question 2')).toBeVisible()).
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/browser/src/__tests__/extensions/surveys/back-button.test.tsx
Line: 101-120
Comment:
**Missing `waitFor` / `async` in test that checks async state**
The test `'back button is hidden when allowGoBack is not set'` is a synchronous test that asserts `screen.getByText('Question 2')` directly after `fireEvent.click`. Every other test in this file that checks post-click state uses `await waitFor(...)` and is `async`. If the Preact state update ever becomes deferred, this test will fail with an unhelpful "element not found" error instead of a meaningful assertion failure. The test should be `async` and use `await waitFor(() => expect(screen.getByText('Question 2')).toBeVisible())`.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+56
to
+198
| describe('Surveys: back button', () => { | ||
| beforeEach(() => { | ||
| cleanup() | ||
| jest.clearAllMocks() | ||
| mockedUuidv7.mockReturnValue('generated-uuid') | ||
| mockedGetInProgressSurveyState.mockReturnValue(null) | ||
| HTMLFormElement.prototype.submit = jest.fn() | ||
| }) | ||
|
|
||
| afterEach(() => { | ||
| delete (HTMLFormElement.prototype as any).submit | ||
| }) | ||
|
|
||
| test('back button is hidden on the first question', () => { | ||
| render( | ||
| <SurveyPopup survey={baseSurvey} removeSurveyFromFocus={jest.fn()} isPopup posthog={mockPosthog as any} /> | ||
| ) | ||
|
|
||
| expect(screen.getByText('Question 1')).toBeVisible() | ||
| expect(screen.queryByRole('button', { name: /go to previous question/i })).not.toBeInTheDocument() | ||
| }) | ||
|
|
||
| test('back button is hidden when allowGoBack is not set', () => { | ||
| const survey = { ...baseSurvey, appearance: { ...baseSurvey.appearance, allowGoBack: false } } | ||
| render(<SurveyPopup survey={survey} removeSurveyFromFocus={jest.fn()} isPopup posthog={mockPosthog as any} />) | ||
|
|
||
| fireEvent.input(screen.getByRole('textbox'), { target: { value: 'a' } }) | ||
| fireEvent.click(screen.getByRole('button', { name: /submit survey/i })) | ||
|
|
||
| expect(screen.getByText('Question 2')).toBeVisible() | ||
| expect(screen.queryByRole('button', { name: /go to previous question/i })).not.toBeInTheDocument() | ||
| }) | ||
|
|
||
| test('back button appears after advancing and returns to the previous question with prior answer prefilled', async () => { | ||
| render( | ||
| <SurveyPopup survey={baseSurvey} removeSurveyFromFocus={jest.fn()} isPopup posthog={mockPosthog as any} /> | ||
| ) | ||
|
|
||
| fireEvent.input(screen.getByRole('textbox'), { target: { value: 'first answer' } }) | ||
| fireEvent.click(screen.getByRole('button', { name: /submit survey/i })) | ||
|
|
||
| await waitFor(() => expect(screen.getByText('Question 2')).toBeVisible()) | ||
| const backButton = screen.getByRole('button', { name: /go to previous question/i }) | ||
| expect(backButton).toBeVisible() | ||
|
|
||
| fireEvent.click(backButton) | ||
|
|
||
| await waitFor(() => expect(screen.getByText('Question 1')).toBeVisible()) | ||
| expect(screen.getByRole('textbox')).toHaveValue('first answer') | ||
| expect(screen.queryByRole('button', { name: /go to previous question/i })).not.toBeInTheDocument() | ||
| }) | ||
|
|
||
| test('navigation history survives a re-render (resume from persisted state)', async () => { | ||
| // First mount: advance Q1 -> Q2, capture whatever state the SDK persisted. | ||
| const { unmount } = render( | ||
| <SurveyPopup survey={baseSurvey} removeSurveyFromFocus={jest.fn()} isPopup posthog={mockPosthog as any} /> | ||
| ) | ||
|
|
||
| fireEvent.input(screen.getByRole('textbox'), { target: { value: 'first answer' } }) | ||
| fireEvent.click(screen.getByRole('button', { name: /submit survey/i })) | ||
| await waitFor(() => expect(screen.getByText('Question 2')).toBeVisible()) | ||
|
|
||
| const persistedState = mockedSetInProgressSurveyState.mock.calls.at(-1)![1] | ||
| unmount() | ||
|
|
||
| // Second mount: feed the captured state back in (simulating a reload). | ||
| mockedGetInProgressSurveyState.mockReturnValue(persistedState) | ||
| render( | ||
| <SurveyPopup survey={baseSurvey} removeSurveyFromFocus={jest.fn()} isPopup posthog={mockPosthog as any} /> | ||
| ) | ||
|
|
||
| // Behavior: we resume on Q2 AND the back button is available because Q1 is in history. | ||
| expect(screen.getByText('Question 2')).toBeVisible() | ||
| const backButton = screen.getByRole('button', { name: /go to previous question/i }) | ||
| fireEvent.click(backButton) | ||
|
|
||
| await waitFor(() => expect(screen.getByText('Question 1')).toBeVisible()) | ||
| expect(screen.getByRole('textbox')).toHaveValue('first answer') | ||
| }) | ||
|
|
||
| test('respects branching: back lands on the actual previous question, not currentIndex - 1', async () => { | ||
| const branchedSurvey: Survey = { | ||
| ...baseSurvey, | ||
| questions: [ | ||
| { | ||
| type: SurveyQuestionType.Open, | ||
| question: 'Question 1', | ||
| id: 'q1', | ||
| branching: { type: SurveyQuestionBranchingType.SpecificQuestion, index: 2 }, | ||
| }, | ||
| { type: SurveyQuestionType.Open, question: 'Question 2', id: 'q2' }, | ||
| { type: SurveyQuestionType.Open, question: 'Question 3', id: 'q3' }, | ||
| ], | ||
| } | ||
|
|
||
| render( | ||
| <SurveyPopup | ||
| survey={branchedSurvey} | ||
| removeSurveyFromFocus={jest.fn()} | ||
| isPopup | ||
| posthog={mockPosthog as any} | ||
| /> | ||
| ) | ||
|
|
||
| fireEvent.input(screen.getByRole('textbox'), { target: { value: 'skip to q3' } }) | ||
| fireEvent.click(screen.getByRole('button', { name: /submit survey/i })) | ||
|
|
||
| await waitFor(() => expect(screen.getByText('Question 3')).toBeVisible()) | ||
| fireEvent.click(screen.getByRole('button', { name: /go to previous question/i })) | ||
|
|
||
| await waitFor(() => expect(screen.getByText('Question 1')).toBeVisible()) | ||
| expect(screen.queryByText('Question 2')).not.toBeInTheDocument() | ||
| }) | ||
|
|
||
| test('resuming an in-progress survey without visitedIndices does not crash and hides back', () => { | ||
| mockedGetInProgressSurveyState.mockReturnValue({ | ||
| surveySubmissionId: 'legacy-id', | ||
| lastQuestionIndex: 1, | ||
| responses: { $survey_response_q1: 'previous answer' }, | ||
| }) | ||
|
|
||
| render( | ||
| <SurveyPopup survey={baseSurvey} removeSurveyFromFocus={jest.fn()} isPopup posthog={mockPosthog as any} /> | ||
| ) | ||
|
|
||
| expect(screen.getByText('Question 2')).toBeVisible() | ||
| expect(screen.queryByRole('button', { name: /go to previous question/i })).not.toBeInTheDocument() | ||
| }) | ||
|
|
||
| test('uses custom backButtonText from appearance', async () => { | ||
| const survey = { | ||
| ...baseSurvey, | ||
| appearance: { ...baseSurvey.appearance, backButtonText: 'Previous' }, | ||
| } | ||
| render(<SurveyPopup survey={survey} removeSurveyFromFocus={jest.fn()} isPopup posthog={mockPosthog as any} />) | ||
|
|
||
| fireEvent.input(screen.getByRole('textbox'), { target: { value: 'a' } }) | ||
| fireEvent.click(screen.getByRole('button', { name: /submit survey/i })) | ||
|
|
||
| await waitFor(() => expect(screen.getByText('Question 2')).toBeVisible()) | ||
| expect(screen.getByRole('button', { name: /go to previous question/i })).toHaveTextContent('Previous') | ||
| }) | ||
| }) |
Contributor
There was a problem hiding this comment.
The seven test cases share the same render setup and differ mainly in the combination of allowGoBack, backButtonText, and whether visitedIndices is present. The project's style guide prefers parameterised tests. Grouping the visibility/hidden-state cases into a test.each table would reduce duplication and make it easier to add future combinations (e.g., allowGoBack: undefined).
Context Used: Do not attempt to comment on incorrect alphabetica... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/browser/src/__tests__/extensions/surveys/back-button.test.tsx
Line: 56-198
Comment:
**Tests are not parameterised (project preference)**
The seven test cases share the same render setup and differ mainly in the combination of `allowGoBack`, `backButtonText`, and whether `visitedIndices` is present. The project's style guide prefers parameterised tests. Grouping the visibility/hidden-state cases into a `test.each` table would reduce duplication and make it easier to add future combinations (e.g., `allowGoBack: undefined`).
**Context Used:** Do not attempt to comment on incorrect alphabetica... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Graphite Automations"sdk release label" took an action on this PR • (05/19/26)1 label was added to this PR based on Adam Bowker's automation. "Add graphite merge queue [copy]" took an action on this PR • (05/19/26)2 labels were added to this PR based on Lucas Faria's automation. |
- RN: add allowGoBack/backButtonText defaults so the new core fields satisfy SurveyAppearanceTheme (root cause for cascading CI failures). - BottomSection: only wrap submit in form-buttons div when the Back button is actually shown — preserves DOM structure for existing surveys. - Prune responses for abandoned branches before sending: if the user backs up and changes an earlier answer that takes a different path, the old branch's answers no longer leak into the submitted payload. - Use waitFor in the negative back-button visibility test. - Add changeset. Generated-By: PostHog Code Task-Id: d229cf67-cbe1-43d1-9b8f-fa32ae50eca6
Contributor
Author
|
Pushed Greptile #1 — DOM structure regression: fixed. Greptile #2 — stale responses on branch change: fixed. On submission (partial and final), we now prune Greptile #3 — missing Greptile #4 — parameterise tests: respectfully pushing back. The 7 cases assert distinct behaviors (visibility, branching, persistence resume, pre-fill, custom label), each with different setup or assertions. A CI failures: all cascaded from one root cause — adding Added a changeset (minor bump for both |
Contributor
Contributor
|
Size Change: +12 kB (+0.07%) Total Size: 16.1 MB
ℹ️ View Unchanged
|
Member
make your review pr workflow to reply directly to the comment thread, this way you have to jump context and the # id is easily out of sequence/outdated |
marandaneto
approved these changes
May 20, 2026
Contributor
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the |
Member
|
@PostHog/team-surveys ping |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a built-in, opt-in Back button to web surveys so respondents can return to a previously visited question and edit their answer. Today the surveys SDK is strictly forward-only — this fills a real UX gap, especially on multi-question and branched surveys.
allowGoBack(defaultfalse) andbackButtonText(defaults to "Back"). Added to@posthog/core'sSurveyAppearanceso the field is discoverable for future RN parity.currentIndex - 1:InProgressSurveyStategains an optionalvisitedIndices: number[]so Back respects the path the user actually took through branching surveys (response_based,specific_question). Existing in-progress state withoutvisitedIndicesresumes safely with the button hidden until the next forward step.initialValuepath — no retyping.allowGoBackis not set (preserves existing behavior for every current survey).Out of scope
packages/react-native/src/surveys/) — separate parity PR, the new appearance fields live on the shared core type so it can land next.PostHogSurveysyet — deferred until a concrete external ask.Test plan
packages/browser/src/__tests__/extensions/surveys/back-button.test.tsx, 7 cases): visibility on first question, hidden when flag unset, advance+back round-trip with pre-fill, history survives re-render from persisted state, branching respected (Q1 → Q3 → back lands on Q1, not Q2), legacy state withoutvisitedIndicesdoesn't crash, custombackButtonTexthonored.packages/browser/playwright/mocked/surveys/back-button.spec.ts, 2 cases): full bundle smoke for the positive and negative path.Created with PostHog Code