Repository navigation
Conditional questions: base implementation plus support for radio and dropdown triggers - #3833
Open
AndyScherzinger wants to merge 38 commits into
Open
AndyScherzinger wants to merge 38 commits into
AndyScherzinger wants to merge 38 commits into
Conversation
This is an implementation of Conditional Logic. It is not well tested, and I would appreciate review and testing by others to see if it works as intended. I have created a few forms and clicked around to the best of my abilities, but I have implemented the fix for the benefit of others that need it and have no real use case myself to test with. Fixes: #358 Signed-off-by: Micke Nordin <kano@sunet.se>
AndyScherzinger
added
enhancement
This was referenced Nov 4, 2026
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
AndyScherzinger
changed the title
Member
Author
|
I'll work on fixing CI checks and test coverage next |
AndyScherzinger
force-pushed
the
feat/conditional-questions-1-base
branch
from
November 4, 2026 14:07
b833f24 to
9d9b414
Compare
Chartman123
reviewed
Nov 4, 2026
Chartman123
left a comment
Chartman123
left a comment
Collaborator
There was a problem hiding this comment.
Added a few general comments/questions.
One thing that came to my mind: would it be easier to not have a new "question type" conditional but to make this part of the props of a "normal" question?
| | `column` | Represents a column header when rendering options in a grid-style question. | | ||
|
|
||
| `row` and `column` option types are primarily used together with `grid` question types to build a two-dimensional selection matrix. `choice` is the default for normal option lists. | ||
| | `triggerType` | `conditional` | string | [See trigger types](#conditional-trigger-types) | The type of trigger question (dropdown, multiple_unique, etc.) | |
Collaborator
There was a problem hiding this comment.
Something seems to be wrong with this addition here, probably misplaced after a rebase?
Collaborator
There was a problem hiding this comment.
Not a question, should be placed somewhere else
| #[CORS()] | ||
| #[NoAdminRequired()] | ||
| #[BruteForceProtection(action: 'form')] | ||
| #[ApiRoute(verb: 'PATCH', url: '/api/v3/forms/{formId}/subquestions')] |
Collaborator
There was a problem hiding this comment.
Not sure about the API path here. What about using /api/v3/forms/{formId}/questions/{parentquestionId}/branches/{branchId}/subquestions to keep our current URL schema? Same of course then for all other sub-question related API routes...
Perhaps we could also skip the branchId in the path.
Collaborator
There was a problem hiding this comment.
also not directly a question but more a helper component, should also be moved somewhere else
fix: hide trigger type header on read fix: hide broken title on trigger question fix: clear button fix: aria labeled by index fix: conditional-file-upload Signed-off-by: TimedIn <git@timedin.net> Signed-off-by: Micke Nordin <kano@sunet.se> Co-authored-by: global-prog <raqeeb@uosamarra.edu.iq> Co-authored-by: Micke Nordin <kano@sunet.se> Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de> Assisted-by: Claude Code:claude-opus-5-5
Signed-off-by: TimedIn <git@timedin.net>
fix: export of subquestions Signed-off-by: TimedIn <git@timedin.net> Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de> Assisted-by: Claude Code:claude-opus-5-5
Signed-off-by: TimedIn <git@timedin.net>
fix: reordering and dragging Signed-off-by: TimedIn <git@timedin.net>
Signed-off-by: TimedIn <git@timedin.net>
Signed-off-by: TimedIn <git@timedin.net>
Signed-off-by: TimedIn <git@timedin.net>
…n' to conditional Signed-off-by: TimedIn <git@timedin.net>
Signed-off-by: TimedIn <git@timedin.net>
- validate active-branch subquestions with full per-question rules - persist subquestion answers only for the activated branch - use -PHP_FLOAT_MAX as the value_range lower bound - drop unused QuestionMapper::deleteByParentQuestion Signed-off-by: Micke Nordin <kano@sunet.se>
Signed-off-by: TimedIn <git@timedin.net>
Signed-off-by: TimedIn <git@timedin.net>
Signed-off-by: TimedIn <git@timedin.net>
Signed-off-by: TimedIn <git@timedin.net>
Signed-off-by: TimedIn <git@timedin.net>
- update test to mock getActiveBranches (plural) - type getActiveBranches/findActiveBranches as non-nullable array - add triggerType and branches to extra settings type - simplify branch guard in question cloning Signed-off-by: Micke Nordin <kano@sunet.se> Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de> Assisted-by: Claude Code:claude-opus-5-5
Signed-off-by: Micke Nordin <kano@sunet.se> Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de> Assisted-by: Claude Code:claude-opus-5-5
Signed-off-by: Micke Nordin <kano@sunet.se>
Signed-off-by: Micke Nordin <kano@sunet.se>
Signed-off-by: Micke Nordin <kano@sunet.se> Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de> Assisted-by: Claude Code:claude-opus-5-5
Signed-off-by: Micke Nordin <kano@sunet.se>
updateQuestion() accepted parentQuestionId and branchId, so a question could be moved under a conditional question of another form. Both are now fixed on creation, like options and accept, which are not columns of a question at all. Changing a conditional question into another type, or a question into a conditional one, would leave subquestions without a parent or nest conditional questions, so these type changes are rejected as well. newQuestion() now also checks that the branch exists on the parent. Subquestions are loaded together with the form's questions in one query, with their options in a second one, instead of two queries per conditional question, and only subquestions of the parent's own form are attached to its branches. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
…sistent - Removing a branch deletes its subquestions instead of leaving them behind as hidden export columns, and the stored branches no longer keep a copy of their subquestions. - Deleting or copying a subquestion renumbers or orders within its branch instead of shifting the top-level questions. - Copying a conditional question, alone or with its form, copies the subquestion rows and points the branch conditions to the options of the copy. The copy used to have empty branches whose conditions referred to the options of the original. - Exports list the subquestions of a conditional question right after it, in branch order, instead of interleaving them by their order. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
…stions The trigger answer of a conditional question skipped every check a regular question gets, and subquestion answers were stored by a copy of the store logic that missed ranking, grid and "other" answers. - The trigger is validated and stored as a question of the trigger type, so unknown options are rejected, an empty trigger does not satisfy a required conditional question and file triggers are kept. - Subquestion answers go through the regular store logic. - Dropdowns send option ids as numbers, which never matched the string comparison, so their branches were never active on the server and the subquestion answers were dropped. - A checkbox branch is active if any of its option combinations is selected, like in the browser. - An unanswered trigger activates no branch. - Regex conditions use the delimited format of the short answer validation, which the server needs, and are checked when saved. Saved conditions must use a condition type of the trigger type. - The unused list of all condition types is removed. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
…tions
Subquestions used the index of the next top-level question, so element
ids were duplicated and inputs got the label of another question.
- Subquestions are numbered after their conditional question ("1.2"),
across all of its branches, and their element ids are derived from
that, the trigger's own elements get ids of their own.
- The trigger is rendered inside the conditional question's list item
instead of as a second list item, and the subquestions shown while
answering are a nested list.
- A polite status message tells screen reader users when follow-up
questions appear or disappear.
- In the editor each branch is a group named after its condition, its
buttons name the branch, the heading follows the question title's
level and names the question, and the condition controls have
labels.
- The unused isTrigger prop is removed.
Assisted-by: Claude Code:claude-opus-5-5
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
… the server - Answers of subquestions in branches that are no longer shown are not sent anymore, the server rejected them as answers for unknown questions. They are kept in the browser in case the trigger is changed back. - Changing the trigger type opens the type selection again instead of saving an empty trigger type, which the server rejected. Settings of the old trigger type are dropped, the conditions of the branches are reset and their subquestions are kept. - Regular expressions are entered as /pattern/modifiers, like for the short answer validation, are checked while typing and matched the same way in the browser as on the server. - Date and time conditions are stored in the answer format instead of as a timestamp string that the server could not read, and compared without the time of day, so a range includes its bounds. - A file trigger without an upload activates the "no file uploaded" branches in the browser as well. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
… its options - The results summary shows the trigger of a conditional question as a question of its trigger type, followed by the subquestions of all branches, which were missing there. - The menu of the trigger only has the options of the trigger type. Required, technical name and delete belong to the conditional question and changed it from a second place, with the trigger always showing as not required. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
Builds a form with a dropdown conditional question through the API and answers it in the browser: only the subquestions of the chosen answer are shown and stored, subquestions are numbered after their question without taking the label or ids of the questions around them. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
The app version is 5.5.0-dev, so the migration that adds the subquestion columns belongs to 5.5 and runs after the existing Version050500Date20261030000000. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
Subquestions were indented by 19px (padding and border) in the editor, the submit view and the results. On narrow screens that width is missed, so there the indentation is only the border and 8px. The border stays, the question number (1.1, 1.2) is only announced to screen readers and not shown. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
The tables listed condition types that do not exist, such as option_not_selected, date_equals or filename_regex, and showed a regular expression without the delimiters the server requires. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
Conditional questions supported eleven trigger types at once. To make the feature reviewable in parts, this base keeps the trigger types that have options: radio buttons and dropdowns, the same as Google Forms offers for branching. The other trigger types are added back one by one on top of this: checkboxes, date and time, file, linear scale and color, and short and long text. Each brings its condition matching on the server and in the browser, its condition editor, docs and tests. Data model, API, copying, export, results, validation and accessibility stay complete here and do not depend on the trigger type. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
Unit tests for reorderSubQuestions(): the new order is stored, and duplicates, a wrong length, unknown questions, questions of another form, parent or branch, deleted questions and archived forms are rejected. A database test for the QuestionMapper queries: top-level questions leave subquestions out, subquestions are found per parent and per branch in their order, and deleted ones only on request. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
The validation finds the answer of a question by its id with array_key_exists, while the check for answers to unknown questions used a loose in_array and the answers are stored after a loose array_search. An answer keyed by another notation of a question id, like "1.0", therefore skipped the validation of that question and was still stored for it. The check for malformed option ids compared the id with itself, so an option id in another notation, like "05", was accepted and stored as that option. Conditional questions match their branches against the option ids as they are, so such a trigger answer stored the option without opening its branch. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
Deleted questions are not exported, their answers are. Importing such an answer failed, as no imported question belongs to it, and stopped the import of the remaining forms. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
Adding a question moves the focus to the title of the new question. Only checkbox and radio questions handed their root element to useQuestion, so for all other types the focus stayed on the closed menu and fell back to the page. Every question component binds its root element now. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
AndyScherzinger
force-pushed
the
feat/conditional-questions-1-base
branch
from
November 4, 2026 16:51
20f7fab to
21f29c0
Compare
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
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.
Conditional questions from #3123 by @mickenordin, with the fixes by @timedin-de and the review fixes, split into a stack so it can be reviewed in parts (as suggested by @Chartman123 in #3123 (comment)).
Fixes #358
Stack
Each PR is based on the one before it. Review and merge from the top.
This PR
A conditional question shows follow-up questions (subquestions) depending on the answer to its trigger. This base allows radio buttons and dropdowns as trigger, the same as Google Forms allows for branching. The other trigger types are added back by the PRs above.
Everything that does not depend on the trigger type is complete here:
parentQuestionIdandbranchId, branches inextraSettings);Follow-ups
Nested conditionals, and/or logic and a default branch are left for later PRs, as listed in #3123 (comment).
🤖 AI (if applicable)