Skip to content

Follow up IME option creation and delete dialog timing fixes #3185

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.

Sign up for GitHub

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

Open
don9x2E wants to merge 2 commits into
base: main
Choose a base branch
from
+169 −117
Open

Follow up IME option creation and delete dialog timing fixes #3185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e41428e
Fix option creation flow during IME composition
don9x2E Feb 26, 2026
327cfc3
Fix lint formatting and update IME E2E expectations
don9x2E Feb 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions playwright/e2e/ime-input.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ test.beforeEach(async ({ page }) => {
})

test(
'IME input does not trigger new option',
'IME input requires explicit intent to create new option',
{
annotation: {
type: 'issue',
Expand Down Expand Up @@ -75,7 +75,12 @@ test(
await client.send('Input.insertText', {
text: 'さ',
})
// so there were 4 inputs but those should only result in one new option
// Committing composition text alone must not create a new option.
await expect(question.answerInputs).toHaveCount(0)
await expect(question.newAnswerInput).toHaveValue('さ')

// Explicit intent (Enter) creates exactly one option.
await question.newAnswerInput.press('Enter')
await expect(question.answerInputs).toHaveCount(1)
await expect(question.answerInputs).toHaveValue('さ')
},
Expand Down
201 changes: 103 additions & 98 deletions src/components/AppNavigationForm.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,104 +4,109 @@
-->

<template>
<NcListItem
:active="isActive"
:actions-aria-label="t('forms', 'Form actions')"
:counter-number="form.submissionCount"
compact
:force-display-actions="forceDisplayActions"
:name="formTitle"
:to="{
name: routerTarget,
params: { hash: form.hash },
}"
@click="mobileCloseNavigation">
<template #icon>
<NcLoadingIcon v-if="loading" :size="16" />
<IconCheck v-else-if="isExpired" :size="16" />
<FormsIcon v-else :size="16" />
</template>
<template v-if="hasSubtitle" #subname>
{{ formSubtitle }}
</template>
<template
v-if="!loading && (!readOnly || canEdit || canSeeResults)"
#actions>
<NcActionRouter
v-if="!isArchived && canEdit"
close-after-click
:disabled="isFormLocked"
exact
:to="{ name: 'edit', params: { hash: form.hash } }"
@click="mobileCloseNavigation">
<template #icon>
<IconPencil :size="20" />
</template>
{{ t('forms', 'Edit form') }}
</NcActionRouter>
<NcActionButton
v-if="!isArchived && !readOnly"
close-after-click
@click="onShareForm">
<template #icon>
<IconShareVariant :size="20" />
</template>
{{ t('forms', 'Share form') }}
</NcActionButton>
<NcActionRouter
v-if="canSeeResults"
close-after-click
exact
:to="{ name: 'results', params: { hash: form.hash } }"
@click="mobileCloseNavigation">
<template #icon>
<IconPoll :size="20" />
</template>
{{ t('forms', 'Results') }}
</NcActionRouter>
<NcActionButton v-if="canEdit" close-after-click @click="onCloneForm">
<template #icon>
<IconContentCopy :size="20" />
</template>
{{ t('forms', 'Copy form') }}
</NcActionButton>
<NcActionSeparator v-if="canEdit && !readOnly" />
<NcActionButton
v-if="canEdit && !readOnly"
close-after-click
:disabled="isFormLocked"
@click="onToggleArchive">
<template #icon>
<IconArchiveOff v-if="isArchived" :size="20" />
<IconArchive v-else :size="20" />
</template>
{{
isArchived
? t('forms', 'Unarchive form')
: t('forms', 'Archive form')
}}
</NcActionButton>
<NcActionButton
v-if="canEdit && !readOnly"
close-after-click
:disabled="isFormLocked"
@click="showDeleteDialog = true">
<template #icon>
<IconDelete :size="20" />
</template>
{{ t('forms', 'Delete form') }}
</NcActionButton>
<NcDialog
:open.sync="showDeleteDialog"
:name="t('forms', 'Delete form')"
:message="
t('forms', 'Are you sure you want to delete {title}?', {
title: formTitle,
})
"
:buttons="buttons" />
</template>
</NcListItem>
<div>
<NcListItem
:active="isActive"
:actions-aria-label="t('forms', 'Form actions')"
:counter-number="form.submissionCount"
compact
:force-display-actions="forceDisplayActions"
:name="formTitle"
:to="{
name: routerTarget,
params: { hash: form.hash },
}"
@click="mobileCloseNavigation">
<template #icon>
<NcLoadingIcon v-if="loading" :size="16" />
<IconCheck v-else-if="isExpired" :size="16" />
<FormsIcon v-else :size="16" />
</template>
<template v-if="hasSubtitle" #subname>
{{ formSubtitle }}
</template>
<template
v-if="!loading && (!readOnly || canEdit || canSeeResults)"
#actions>
<NcActionRouter
v-if="!isArchived && canEdit"
close-after-click
:disabled="isFormLocked"
exact
:to="{ name: 'edit', params: { hash: form.hash } }"
@click="mobileCloseNavigation">
<template #icon>
<IconPencil :size="20" />
</template>
{{ t('forms', 'Edit form') }}
</NcActionRouter>
<NcActionButton
v-if="!isArchived && !readOnly"
close-after-click
@click="onShareForm">
<template #icon>
<IconShareVariant :size="20" />
</template>
{{ t('forms', 'Share form') }}
</NcActionButton>
<NcActionRouter
v-if="canSeeResults"
close-after-click
exact
:to="{ name: 'results', params: { hash: form.hash } }"
@click="mobileCloseNavigation">
<template #icon>
<IconPoll :size="20" />
</template>
{{ t('forms', 'Results') }}
</NcActionRouter>
<NcActionButton
v-if="canEdit"
close-after-click
@click="onCloneForm">
<template #icon>
<IconContentCopy :size="20" />
</template>
{{ t('forms', 'Copy form') }}
</NcActionButton>
<NcActionSeparator v-if="canEdit && !readOnly" />
<NcActionButton
v-if="canEdit && !readOnly"
close-after-click
:disabled="isFormLocked"
@click="onToggleArchive">
<template #icon>
<IconArchiveOff v-if="isArchived" :size="20" />
<IconArchive v-else :size="20" />
</template>
{{
isArchived
? t('forms', 'Unarchive form')
: t('forms', 'Archive form')
}}
</NcActionButton>
<NcActionButton
v-if="canEdit && !readOnly"
close-after-click
:disabled="isFormLocked"
@click="showDeleteDialog = true">
<template #icon>
<IconDelete :size="20" />
</template>
{{ t('forms', 'Delete form') }}
</NcActionButton>
</template>
</NcListItem>
<NcDialog
:open.sync="showDeleteDialog"
:name="t('forms', 'Delete form')"
:message="
t('forms', 'Are you sure you want to delete {title}?', {
title: formTitle,
})
"
:buttons="buttons" />
</div>
</template>

<script>
Expand Down
73 changes: 57 additions & 16 deletions src/components/Questions/AnswerInput.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
dir="auto"
@input="debounceOnInput"
@keydown.delete="deleteEntry"
@keydown.enter.prevent="focusNextInput"
@keydown.enter.prevent="onEnter"
@compositionstart="onCompositionStart"
@compositionend="onCompositionEnd" />

Expand Down Expand Up @@ -64,6 +64,17 @@
</template>
</NcButton>
</div>
<div v-else class="option__actions">
<NcButton
:aria-label="t('forms', 'Add a new answer option')"
variant="tertiary"
:disabled="isIMEComposing || !canCreateLocalAnswer"
@click="createLocalAnswer">
<template #icon>
<IconPlus :size="20" />
</template>
</NcButton>
</div>
</li>
</template>

Expand Down Expand Up @@ -98,6 +109,7 @@
IconCheckboxBlankOutline,
IconDelete,
IconDragIndicator,
IconPlus,
IconRadioboxBlank,
IconTableColumn,
IconTableRow,
Expand Down Expand Up @@ -167,6 +179,10 @@
},

computed: {
canCreateLocalAnswer() {
return !!this.$refs.input?.value?.trim()
},

ariaLabel() {
if (this.answer.local) {
if (this.optionType === OptionType.Column) {
Expand Down Expand Up @@ -266,33 +282,58 @@
* @param {InputEvent} event The input event that triggered adding a new entry
*/
async onInput({ target, isComposing }) {
if (this.answer.local) {
this.$set(this.answer, 'text', target.value)
return
}

if (!isComposing && !this.isIMEComposing && target.value !== '') {
// clone answer
const answer = { ...this.answer }
answer.text = this.$refs.input.value

if (this.answer.local) {
// Dispatched for creation. Marked as synced
this.$set(this.answer, 'local', false)
const newAnswer = await this.createAnswer(answer)
await this.updateAnswer(answer)

// Forward changes, but use current answer.text to avoid erasing
// any in-between changes while creating the answer
newAnswer.text = this.$refs.input.value
// Forward changes, but use current answer.text to avoid erasing
// any in-between changes while updating the answer
answer.text = this.$refs.input.value
this.$emit('update:answer', this.index, answer)
}
},

this.$emit('create-answer', this.index, newAnswer)
} else {
await this.updateAnswer(answer)
onEnter(e) {
if (this.answer.local) {
this.createLocalAnswer(e)
return
}
this.focusNextInput(e)
},

// Forward changes, but use current answer.text to avoid erasing
// any in-between changes while updating the answer
answer.text = this.$refs.input.value
this.$emit('update:answer', this.index, answer)
}
async createLocalAnswer(e) {
if (this.isIMEComposing || e?.isComposing) {
return
}

const value = this.$refs.input?.value ?? ''
if (!value.trim()) {
return
}

const answer = { ...this.answer }
answer.text = value

// Dispatched for creation. Marked as synced
this.$set(this.answer, 'local', false)
const newAnswer = await this.createAnswer(answer)

// Forward changes, but use current answer.text to avoid erasing
// any in-between changes while creating the answer
newAnswer.text = this.$refs.input.value

this.$emit('create-answer', this.index, newAnswer)
},

/**

Check warning on line 336 in src/components/Questions/AnswerInput.vue

View workflow job for this annotation

GitHub Actions / NPM lint 

Missing JSDoc @param "e" declaration
* Request a new answer
*/
focusNextInput(e) {
Expand Down
3 changes: 2 additions & 1 deletion src/mixins/QuestionMultipleMixin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@
)
},

/**

Check warning on line 170 in src/mixins/QuestionMultipleMixin.ts

View workflow job for this annotation

GitHub Actions / NPM lint 

JSDoc @return declaration present but return expression not available in function
* Handles the creation of a new answer option.
*
* @param index the index of the answer
Expand All @@ -176,7 +176,8 @@
*/
onCreateAnswer(index: number, answer: FormsOption): void {
this.$nextTick(() => {
this.$nextTick(() => this.focusIndex(index, answer.optionType))
// Move focus to the newly appended empty local option.
this.$nextTick(() => this.focusIndex(index + 1, answer.optionType))
})
this.updateOptions([...this.options, answer])
},
Expand Down Expand Up @@ -232,7 +233,7 @@
/**
* Remove any empty options when leaving an option
*
* @param optionType

Check warning on line 236 in src/mixins/QuestionMultipleMixin.ts

View workflow job for this annotation

GitHub Actions / NPM lint 

Missing JSDoc @param "optionType" description
*/
checkValidOption(optionType: string) {
// When leaving edit mode, filter and delete empty options
Expand Down
Loading