Skip to content

Commit 31a152f

Browse files
authored
Merge pull request #3200 from nextcloud/backport/3185/stable5.2
[stable5.2] Follow up IME option creation and delete dialog timing fixes
2 parents 05b26ec + e98a3ac commit 31a152f

File tree

3 files changed

+95
-20
lines changed

3 files changed

+95
-20
lines changed

playwright/e2e/ime-input.spec.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ test.beforeEach(async ({ page }) => {
1717
})
1818

1919
test(
20-
'IME input does not trigger new option',
20+
'IME input requires explicit intent to create new option',
2121
{
2222
annotation: {
2323
type: 'issue',
@@ -75,7 +75,12 @@ test(
7575
await client.send('Input.insertText', {
7676
text: 'さ',
7777
})
78-
// so there were 4 inputs but those should only result in one new option
78+
// Committing composition text alone must not create a new option.
79+
await expect(question.answerInputs).toHaveCount(0)
80+
await expect(question.newAnswerInput).toHaveValue('さ')
81+
82+
// Explicit intent (Enter) creates exactly one option.
83+
await question.newAnswerInput.press('Enter')
7984
await expect(question.answerInputs).toHaveCount(1)
8085
await expect(question.answerInputs).toHaveValue('さ')
8186
},

src/components/Questions/AnswerInput.vue

Lines changed: 87 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -11,17 +11,17 @@
1111
class="question__item__pseudoInput" />
1212
<input
1313
ref="input"
14+
v-model="localText"
1415
:aria-label="ariaLabel"
1516
:placeholder="placeholder"
16-
:value="answer.text"
1717
class="question__input"
1818
:class="{ 'question__input--shifted': !isDropdown }"
1919
:maxlength="maxOptionLength"
2020
type="text"
2121
dir="auto"
2222
@input="debounceOnInput"
2323
@keydown.delete="deleteEntry"
24-
@keydown.enter.prevent="focusNextInput"
24+
@keydown.enter.prevent="onEnter"
2525
@compositionstart="onCompositionStart"
2626
@compositionend="onCompositionEnd" />
2727

@@ -64,6 +64,17 @@
6464
</template>
6565
</NcButton>
6666
</div>
67+
<div v-else class="option__actions">
68+
<NcButton
69+
:aria-label="t('forms', 'Add a new answer option')"
70+
variant="tertiary"
71+
:disabled="isIMEComposing || !canCreateLocalAnswer"
72+
@click="createLocalAnswer">
73+
<template #icon>
74+
<IconPlus :size="20" />
75+
</template>
76+
</NcButton>
77+
</div>
6778
</li>
6879
</template>
6980

@@ -79,6 +90,7 @@ import IconArrowUp from 'vue-material-design-icons/ArrowUp.vue'
7990
import IconCheckboxBlankOutline from 'vue-material-design-icons/CheckboxBlankOutline.vue'
8091
import IconDelete from 'vue-material-design-icons/TrashCanOutline.vue'
8192
import IconDragIndicator from '../Icons/IconDragIndicator.vue'
93+
import IconPlus from 'vue-material-design-icons/Plus.vue'
8294
import IconRadioboxBlank from 'vue-material-design-icons/RadioboxBlank.vue'
8395
8496
import NcActions from '@nextcloud/vue/components/NcActions'
@@ -98,6 +110,7 @@ export default {
98110
IconCheckboxBlankOutline,
99111
IconDelete,
100112
IconDragIndicator,
113+
IconPlus,
101114
IconRadioboxBlank,
102115
NcActions,
103116
NcActionButton,
@@ -140,10 +153,18 @@ export default {
140153
queue: null,
141154
debounceOnInput: null,
142155
isIMEComposing: false,
156+
localText: this.answer?.text ?? '',
143157
}
144158
},
145159
146160
computed: {
161+
canCreateLocalAnswer() {
162+
if (this.answer.local) {
163+
return !!this.localText?.trim()
164+
}
165+
return !!this.answer.text?.trim()
166+
},
167+
147168
ariaLabel() {
148169
if (this.answer.local) {
149170
return t('forms', 'Add a new answer option')
@@ -169,6 +190,17 @@ export default {
169190
},
170191
},
171192
193+
watch: {
194+
// Keep localText in sync when the parent replaces/updates the answer prop
195+
answer: {
196+
handler(newVal) {
197+
this.localText = newVal?.text ?? ''
198+
},
199+
200+
deep: true,
201+
},
202+
},
203+
172204
created() {
173205
this.queue = new PQueue({ concurrency: 1 })
174206
@@ -196,34 +228,72 @@ export default {
196228
* @param {InputEvent} event The input event that triggered adding a new entry
197229
*/
198230
async onInput({ target, isComposing }) {
231+
if (this.answer.local) {
232+
this.localText = target.value
233+
return
234+
}
235+
199236
if (!isComposing && !this.isIMEComposing && target.value !== '') {
200237
// clone answer
201238
const answer = Object.assign({}, this.answer)
202239
answer.text = this.$refs.input.value
203240
204-
if (this.answer.local) {
205-
// Dispatched for creation. Marked as synced
206-
this.$set(this.answer, 'local', false)
207-
const newAnswer = await this.createAnswer(answer)
241+
await this.updateAnswer(answer)
242+
243+
// Forward changes, but use current answer.text to avoid erasing
244+
// any in-between changes while updating the answer
245+
answer.text = this.$refs.input.value
246+
this.$emit('update:answer', this.index, answer)
247+
}
248+
},
208249
209-
// Forward changes, but use current answer.text to avoid erasing
210-
// any in-between changes while creating the answer
211-
newAnswer.text = this.$refs.input.value
250+
/**
251+
* Handle Enter key: create local answer or move focus
252+
*
253+
* @param {KeyboardEvent} e the keydown event
254+
*/
255+
onEnter(e) {
256+
if (this.answer.local) {
257+
this.createLocalAnswer(e)
258+
return
259+
}
260+
this.focusNextInput(e)
261+
},
212262
213-
this.$emit('create-answer', this.index, newAnswer)
214-
} else {
215-
await this.updateAnswer(answer)
263+
/**
264+
* Create a new local answer option from the current input
265+
*
266+
* @param {Event} e the triggering event
267+
*/
268+
async createLocalAnswer(e) {
269+
if (this.isIMEComposing || e?.isComposing) {
270+
return
271+
}
216272
217-
// Forward changes, but use current answer.text to avoid erasing
218-
// any in-between changes while updating the answer
219-
answer.text = this.$refs.input.value
220-
this.$emit('update:answer', this.index, answer)
221-
}
273+
const value = this.localText ?? ''
274+
if (!value.trim()) {
275+
return
222276
}
277+
278+
const answer = { ...this.answer }
279+
answer.text = value
280+
281+
// Dispatched for creation. Marked as synced
282+
this.$set(this.answer, 'local', false)
283+
const newAnswer = await this.createAnswer(answer)
284+
285+
// Forward changes, but use current answer.text to avoid erasing
286+
// any in-between changes while creating the answer
287+
newAnswer.text = this.$refs.input.value
288+
this.localText = ''
289+
290+
this.$emit('create-answer', this.index, newAnswer)
223291
},
224292
225293
/**
226294
* Request a new answer
295+
*
296+
* @param {Event} e the triggering event
227297
*/
228298
focusNextInput(e) {
229299
if (this.isIMEComposing || e?.isComposing) {

src/mixins/QuestionMultipleMixin.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ export default defineComponent({
127127
*/
128128
onCreateAnswer(index: number, answer: FormsOption): void {
129129
this.$nextTick(() => {
130-
this.$nextTick(() => this.focusIndex(index))
130+
this.$nextTick(() => this.focusIndex(index + 1))
131131
})
132132
this.updateOptions([...this.options, answer])
133133
},

0 commit comments

Comments
 (0)