Skip to content

Commit 5a68239

Browse files
authored
Merge pull request #3185 from don9x2E/fix/ime-option-creation-followup
Follow up IME option creation and delete dialog timing fixes
2 parents b68b1b6 + e92a705 commit 5a68239

3 files changed

Lines changed: 95 additions & 20 deletions

File tree

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: 86 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

@@ -98,6 +109,7 @@ export default {
98109
IconCheckboxBlankOutline,
99110
IconDelete,
100111
IconDragIndicator,
112+
IconPlus,
101113
IconRadioboxBlank,
102114
IconTableColumn,
103115
IconTableRow,
@@ -163,10 +175,18 @@ export default {
163175
queue: null,
164176
debounceOnInput: null,
165177
isIMEComposing: false,
178+
localText: this.answer?.text ?? '',
166179
}
167180
},
168181
169182
computed: {
183+
canCreateLocalAnswer() {
184+
if (this.answer.local) {
185+
return !!this.localText?.trim()
186+
}
187+
return !!this.answer.text?.trim()
188+
},
189+
170190
ariaLabel() {
171191
if (this.answer.local) {
172192
if (this.optionType === OptionType.Column) {
@@ -239,6 +259,17 @@ export default {
239259
},
240260
},
241261
262+
watch: {
263+
// Keep localText in sync when the parent replaces/updates the answer prop
264+
answer: {
265+
handler(newVal) {
266+
this.localText = newVal?.text ?? ''
267+
},
268+
269+
deep: true,
270+
},
271+
},
272+
242273
created() {
243274
this.queue = new PQueue({ concurrency: 1 })
244275
@@ -266,34 +297,72 @@ export default {
266297
* @param {InputEvent} event The input event that triggered adding a new entry
267298
*/
268299
async onInput({ target, isComposing }) {
300+
if (this.answer.local) {
301+
this.localText = target.value
302+
return
303+
}
304+
269305
if (!isComposing && !this.isIMEComposing && target.value !== '') {
270306
// clone answer
271307
const answer = { ...this.answer }
272308
answer.text = this.$refs.input.value
273309
274-
if (this.answer.local) {
275-
// Dispatched for creation. Marked as synced
276-
this.$set(this.answer, 'local', false)
277-
const newAnswer = await this.createAnswer(answer)
310+
await this.updateAnswer(answer)
278311
279-
// Forward changes, but use current answer.text to avoid erasing
280-
// any in-between changes while creating the answer
281-
newAnswer.text = this.$refs.input.value
312+
// Forward changes, but use current answer.text to avoid erasing
313+
// any in-between changes while updating the answer
314+
answer.text = this.$refs.input.value
315+
this.$emit('update:answer', this.index, answer)
316+
}
317+
},
282318
283-
this.$emit('create-answer', this.index, newAnswer)
284-
} else {
285-
await this.updateAnswer(answer)
319+
/**
320+
* Handle Enter key: create local answer or move focus
321+
*
322+
* @param {KeyboardEvent} e the keydown event
323+
*/
324+
onEnter(e) {
325+
if (this.answer.local) {
326+
this.createLocalAnswer(e)
327+
return
328+
}
329+
this.focusNextInput(e)
330+
},
286331
287-
// Forward changes, but use current answer.text to avoid erasing
288-
// any in-between changes while updating the answer
289-
answer.text = this.$refs.input.value
290-
this.$emit('update:answer', this.index, answer)
291-
}
332+
/**
333+
* Create a new local answer option from the current input
334+
*
335+
* @param {Event} e the triggering event
336+
*/
337+
async createLocalAnswer(e) {
338+
if (this.isIMEComposing || e?.isComposing) {
339+
return
340+
}
341+
342+
const value = this.localText ?? ''
343+
if (!value.trim()) {
344+
return
292345
}
346+
347+
const answer = { ...this.answer }
348+
answer.text = value
349+
350+
// Dispatched for creation. Marked as synced
351+
this.$set(this.answer, 'local', false)
352+
const newAnswer = await this.createAnswer(answer)
353+
354+
// Forward changes, but use current answer.text to avoid erasing
355+
// any in-between changes while creating the answer
356+
newAnswer.text = this.$refs.input.value
357+
this.localText = ''
358+
359+
this.$emit('create-answer', this.index, newAnswer)
293360
},
294361
295362
/**
296363
* Request a new answer
364+
*
365+
* @param {Event} e the triggering event
297366
*/
298367
focusNextInput(e) {
299368
if (this.isIMEComposing || e?.isComposing) {

src/mixins/QuestionMultipleMixin.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,8 @@ export default defineComponent({
176176
*/
177177
onCreateAnswer(index: number, answer: FormsOption): void {
178178
this.$nextTick(() => {
179-
this.$nextTick(() => this.focusIndex(index, answer.optionType))
179+
// Move focus to the newly appended empty local option.
180+
this.$nextTick(() => this.focusIndex(index + 1, answer.optionType))
180181
})
181182
this.updateOptions([...this.options, answer])
182183
},

0 commit comments

Comments
 (0)