Skip to content

Commit

Permalink
fix(Submit): Make answers reactive and fix invalid mutation of comp…
Browse files Browse the repository at this point in the history 
…uted property

Setting `answers` using `.sync` is not reactive as property changes are not triggering the computed / watch effect,
so instead use a `onUpdate` method to update the `answers` object in a way that will trigger any watch effect.

Also make `getFormValuesFromLocalStorage` a method instead a computed value.
Previously that computed value were assigned a new value which is invalid as computed values should be constant (except for computed setter but this was none).

And make sure "other" answer is set correctly so it will be saved in LocalStorage

Signed-off-by: Ferdinand Thiessen <[email protected]>
  • Loading branch information
@susnux
susnux committed Nov 22, 2023
1 parent c2cc73e commit 31007d4
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 71 deletions.
89 changes: 47 additions & 42 deletions src/components/Questions/QuestionMultiple.vue
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
</NcCheckboxRadioSwitch>
<div v-if="allowOtherAnswer" class="question__other-answer">
<NcCheckboxRadioSwitch :checked="questionValues"
:value="valueOtherAnswer"
:value="otherAnswer ?? QUESTION_EXTRASETTINGS_OTHER_PREFIX"
:name="`${id}-answer`"
:type="isUnique ? 'radio' : 'checkbox'"
:required="checkRequired('other-answer')"
Expand All @@ -61,10 +61,11 @@
@keydown.enter.exact.prevent="onKeydownEnter">
{{ t('forms', 'Other:') }}
</NcCheckboxRadioSwitch>
<NcInputField :label="placeholderOtherAnswer"
:required="hasRequiredOtherAnswerInput"
:value.sync="inputOtherAnswer"
class="question__input" />
<NcInputField class="question__input"
:label="placeholderOtherAnswer"
:required="otherAnswer !== undefined"
:value="otherAnswerText"
@update:value="onChangeOther" />
</div>
</fieldset>
</template>
Expand Down Expand Up @@ -97,9 +98,9 @@
<div :is="pseudoIcon" class="question__item__pseudoInput" />
<input ref="pseudoInput"
v-model="inputValue"
class="question__input"
:aria-label="t('forms', 'Add a new answer')"
:placeholder="t('forms', 'Add a new answer')"
class="question__input"
:maxlength="maxStringLengths.optionText"
minlength="1"
type="text"
Expand All @@ -126,6 +127,8 @@ import QuestionMixin from '../../mixins/QuestionMixin.js'
import GenRandomId from '../../utils/GenRandomId.js'
import logger from '../../utils/Logger.js'
const QUESTION_EXTRASETTINGS_OTHER_PREFIX = 'system-other-answer:'
export default {
name: 'QuestionMultiple',
Expand All @@ -142,9 +145,7 @@ export default {
data() {
return {
inputOtherAnswer: this.valueToInputOtherAnswer(),
QUESTION_EXTRASETTINGS_OTHER_PREFIX: 'system-other-answer:',
inputValue: '',
QUESTION_EXTRASETTINGS_OTHER_PREFIX,
}
},
Expand All @@ -155,7 +156,7 @@ export default {
isLastEmpty() {
const value = this.options[this.options.length - 1]
return value?.text?.trim().length === 0
return value?.text?.trim?.().length === 0
},
isUnique() {
Expand Down Expand Up @@ -197,29 +198,21 @@ export default {
return this.extraSettings?.allowOtherAnswer
},
valueOtherAnswer() {
return this.QUESTION_EXTRASETTINGS_OTHER_PREFIX + this.inputOtherAnswer
},
hasRequiredOtherAnswerInput() {
const checkedOtherAnswer = this.values.filter(item => item.startsWith(this.QUESTION_EXTRASETTINGS_OTHER_PREFIX))
return checkedOtherAnswer[0] !== undefined
},
},
watch: {
inputOtherAnswer() {
if (this.isUnique) {
this.onChange(this.valueOtherAnswer)
return
}
const values = this.values.filter(item => !item.startsWith(this.QUESTION_EXTRASETTINGS_OTHER_PREFIX))
if (this.inputOtherAnswer !== '') {
values.push(this.valueOtherAnswer)
/**
* Text of the "other" answer field
*/
otherAnswerText() {
if (this.otherAnswer) {
return this.otherAnswer.slice(QUESTION_EXTRASETTINGS_OTHER_PREFIX.length)
}
return ''
},
this.onChange(values)
/**
* The full "other" answer including prefix, undefined if no "other answer"
*/
otherAnswer() {
return this.values.find((v) => v.startsWith(QUESTION_EXTRASETTINGS_OTHER_PREFIX))
},
},
Expand All @@ -228,6 +221,20 @@ export default {
this.$emit('update:values', this.isUnique ? [value] : value)
},
/**
* Called when the value of the "other" anwer is changed input
* @param {string} value the new text of the "other" answer
*/
onChangeOther(value) {
// Prefix the value
const prefixedValue = `${QUESTION_EXTRASETTINGS_OTHER_PREFIX}${value}`
// emit the values and add the "other" answer
this.$emit(
'update:values',
this.isUnique ? [prefixedValue] : [...this.values.filter((v) => !v.startsWith(QUESTION_EXTRASETTINGS_OTHER_PREFIX)), prefixedValue],
)
},
/**
* Is the provided answer required ?
* This is needed for checkboxes as html5
Expand Down Expand Up @@ -280,6 +287,7 @@ export default {
/**
* Update the options
* This will handle updating the form (emitting the changes) and update last changed property
*
* @param {Array} options options to change
*/
Expand All @@ -304,22 +312,24 @@ export default {
/**
* Add a new empty answer locally
* @param {InputEvent} event The input event that triggered adding a new entry
*/
addNewEntry() {
addNewEntry({ target }) {
// Add local entry
const options = this.options.slice()
options.push({
id: GenRandomId(),
questionId: this.id,
text: this.inputValue,
text: target?.value ?? '',
local: true,
})
this.inputValue = ''
// Update question
// Update questions
this.updateOptions(options)
// Reset the "new answer" input
this.$refs.pseudoInput.value = ''
this.$nextTick(() => {
this.focusIndex(options.length - 1)
Expand Down Expand Up @@ -412,14 +422,9 @@ export default {
*
* @param {boolean} allowOtherAnswer show/hide field for other answer
*/
onAllowOtherAnswerChange(allowOtherAnswer) {
onAllowOtherAnswerChange(allowOtherAnswer) {
return this.onExtraSettingsChange('allowOtherAnswer', allowOtherAnswer)
},
valueToInputOtherAnswer() {
const otherAnswer = this.values.filter(item => item.startsWith(this.QUESTION_EXTRASETTINGS_OTHER_PREFIX))
return otherAnswer[0] !== undefined ? otherAnswer[0].substring(this.QUESTION_EXTRASETTINGS_OTHER_PREFIX.length) : ''
},
},
}
</script>
Expand Down
90 changes: 61 additions & 29 deletions src/views/Submit.vue
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,11 @@
:answer-type="answerTypes[question.type]"
:index="index + 1"
:max-string-lengths="maxStringLengths"
:values="answers[question.id]"
v-bind="question"
:values.sync="answers[question.id]"
@keydown.enter="onKeydownEnter"
@keydown.ctrl.enter="onKeydownCtrlEnter"
@update:values="addFormFieldToLocalStorage(question)" />
@update:values="(values) => onUpdate(question, values)" />
</ul>
<input ref="submitButton"
class="primary"
Expand Down Expand Up @@ -202,13 +202,6 @@ export default {
},
computed: {
formValuesForLocalStorage() {
const fromLocalStorage = localStorage.getItem(`nextcloud_forms_${this.publicView ? this.shareHash : this.hash}`)
if (fromLocalStorage) {
return JSON.parse(fromLocalStorage)
}
return {}
},
validQuestions() {
return this.form.questions.filter(question => {
// All questions must have a valid title
Expand Down Expand Up @@ -279,7 +272,7 @@ export default {
this.resetData()
// Fetch full form on change
this.fetchFullForm(this.form.id)
this.initFromLocalHost()
this.initFromLocalStorage()
SetWindowTitle(this.formTitle)
},
},
Expand All @@ -300,15 +293,30 @@ export default {
}
SetWindowTitle(this.formTitle)
if (this.isLoggedIn) {
this.initFromLocalHost()
this.initFromLocalStorage()
}
},
methods: {
initFromLocalHost() {
if (localStorage.getItem(`nextcloud_forms_${this.publicView ? this.shareHash : this.hash}`)) {
for (const key in this.formValuesForLocalStorage) {
const answer = this.formValuesForLocalStorage[key]
/**
* Load saved values for current form from LocalStorage
* @return {Record<string,any>}
*/
getFormValuesFromLocalStorage() {
const fromLocalStorage = localStorage.getItem(`nextcloud_forms_${this.publicView ? this.shareHash : this.hash}`)
if (fromLocalStorage) {
return JSON.parse(fromLocalStorage)
}
return null
},
/**
* Initialize answers from saved state in LocalStorage
*/
initFromLocalStorage() {
const savedState = this.getFormValuesFromLocalStorage()
if (savedState) {
for (const [key, answer] of Object.entries(savedState)) {
const answers = []
switch (answer?.type) {
case 'QuestionMultiple':
Expand All @@ -324,6 +332,44 @@ export default {
}
}
},
/**
* Save updated answers for question to LocalStorage in case of browser crash / closes / etc
* @param {*} question Question to update
*/
addFormFieldToLocalStorage(question) {
if (!this.isLoggedIn) {
return
}
// We make sure the values are updated by the `values.sync` handler
const state = {
...(this.getFormValuesFromLocalStorage() ?? {}),
[`${question.id}`]: {
value: this.answers[question.id],
type: answerTypes[question.type].component.name,
},
}
const stringified = JSON.stringify(state)
localStorage.setItem(`nextcloud_forms_${this.publicView ? this.shareHash : this.hash}`, stringified)
},
deleteFormFieldFromLocalStorage() {
if (!this.isLoggedIn) {
return
}
localStorage.removeItem(`nextcloud_forms_${this.publicView ? this.shareHash : this.hash}`)
},
/**
* Update answers of a give value
* @param {{id: number}} question The question to answer
* @param {unknown[]} values The new values
*/
onUpdate(question, values) {
this.answers = { ...this.answers, [question.id]: values }
this.addFormFieldToLocalStorage(question)
},
/**
* On Enter, focus next form-element
* Last form element is the submit button, the form submits on enter then
Expand All @@ -346,20 +392,6 @@ export default {
// Using button-click event to not bypass validity-checks and use our specified behaviour
this.$refs.submitButton.click()
},
addFormFieldToLocalStorage(question) {
if (!this.isLoggedIn) {
return
}
this.formValuesForLocalStorage[`${question.id}`] = { value: this.answers[question.id], type: answerTypes[question.type].component.name }
const parsed = JSON.stringify(this.formValuesForLocalStorage)
localStorage.setItem(`nextcloud_forms_${this.publicView ? this.shareHash : this.hash}`, parsed)
},
deleteFormFieldFromLocalStorage() {
if (!this.isLoggedIn) {
return
}
localStorage.removeItem(`nextcloud_forms_${this.publicView ? this.shareHash : this.hash}`)
},
/*
* Methods for catching unwanted unload events
Expand Down

0 comments on commit 31007d4

Please sign in to comment.