-
Notifications
You must be signed in to change notification settings - Fork 16
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: allow arbitrary response for suggester #900
base: next
Are you sure you want to change the base?
Conversation
39e02a5
to
8aaf8b8
Compare
8aaf8b8
to
9966148
Compare
Quality Gate passedIssues Measures |
|
/* | ||
* Get the list of arbitrary variables. | ||
* An arbitrary variable is the "ArbitraryResponse" object of a question, | ||
* adding "arbitraryOfVariable" which is the corresponding Response variable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
arbitraryOfVariable
sounds weird and unclear.
We could replace it by arbitraryResponseVariableId
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah that was tricky to name it.
arbitraryResponseVariableId
is maybe better, but it's also confusing as we can think it is the id of the arbitrary response variable, not the "main" response variable" :-D
@@ -629,6 +689,36 @@ function getClarificationResponseTableQuestion( | |||
}; | |||
} | |||
|
|||
function getArbitraryResponse(collectedVariablesStore, collectedVariables) { | |||
const collectedvariablequestion = []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
camelCase
Should be plural since it's a list.
src/constants/dictionary.ts
Outdated
@@ -889,6 +889,10 @@ const dictionary: Dictionary = { | |||
fr: 'Retrouver dans le questionnaire', | |||
en: 'Retrieve in the questionnaire', | |||
}, | |||
allowArbitraryResponse: { | |||
fr: 'Autoriser une réponse libre', | |||
en: 'Allow arbitrary response', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
en: 'Allow an arbitrary response',
@@ -10,12 +10,14 @@ import { | |||
const { RADIO } = DATATYPE_VIS_HINT; | |||
|
|||
export const defaultState = { | |||
allowArbitrary: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be clearer as allowArbitraryResponse
.
It could be an arbitrary label or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good idea.
And even in the model we could rename it
For suggester question only outside table, add the possibility of chosing to allow an arbritrary response.
It is quite close to clarification question for code lists, but instead of adding a list ClarificationQuestion in the question, we create a single object ArbitraryResponse.
${questionName}_ARBITRARY
, and its label is${questionName}_ARBITRARY label