Added draft state handling #1956

tammi-23 · 2026-02-09T20:51:37Z

implement #1480

Copilot

Pull request overview

Implements draft state handling for the mail compose flow, including save/discard behaviors and draft mailbox discovery.

Changes:

Added a draft API connector + composable to create/replace/delete drafts and to manage dirty/saving state.
Refactored Pinia stores to sync selected IDs with route query parameters via a shared helper.
Updated UI to prompt on close, auto-save drafts periodically, and show a “Saved” hint.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
packages/web-app-mail/src/helpers/mailDraftConnector.ts	Adds an HTTP-backed connector implementing the draft API.
packages/web-app-mail/src/composables/useSaveAsDraft.ts	Introduces draft-saving/discarding composable with dirty/saving state.
packages/web-app-mail/src/composables/piniaStores/mails.ts	Refactors selected mail handling to use route query binding helper.
packages/web-app-mail/src/composables/piniaStores/mailboxes.ts	Refactors mailboxes store and adds computed drafts mailbox selection.
packages/web-app-mail/src/composables/piniaStores/accounts.ts	Refactors accounts store and binds current account to route query.
packages/web-app-mail/src/composables/piniaStores/helpers.ts	Adds helper to normalize route query values into a single string.
packages/web-app-mail/src/components/MailboxTree.vue	Fixes mailbox selection flow to avoid deref’ing missing account.
packages/web-app-mail/src/components/MailWidget.vue	Adds leave-confirm modal, auto-save, and draft integration to compose modal.
packages/web-app-mail/src/components/MailList.vue	Mounts compose widget only when open; fixes seen-flag update source.
packages/web-app-mail/src/components/MailListItem.vue	Sanitizes HTML-like preview content before rendering preview text.
packages/web-app-mail/src/components/MailComposeForm.vue	Stabilizes props defaults + fixes modelValue usage after toRefs().

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

packages/web-app-mail/src/components/MailWidget.vue

packages/web-app-mail/src/composables/piniaStores/mailboxes.ts

packages/web-app-mail/src/composables/piniaStores/accounts.ts

packages/web-app-mail/src/components/MailWidget.vue

packages/web-app-mail/src/composables/piniaStores/mailboxes.ts

packages/web-app-mail/src/helpers/mailDraftConnector.ts

Co-authored-by: Copilot <[email protected]>

Copilot

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

packages/web-app-mail/src/components/MailWidget.vue

packages/web-app-mail/src/composables/useAutoSaveDraft.ts

packages/web-app-mail/src/composables/piniaStores/helpers.ts

packages/web-app-mail/src/composables/piniaStores/mails.ts

AlexAndBear · 2026-02-11T09:03:27Z

packages/web-app-mail/src/components/MailboxTree.vue

+  const accountId = unref(currentAccount)?.accountId
+  if (!accountId) {
+    return
+  }


We don't need to check this here, if we are selecting a mailbox and no account is set, this is a bug, which might break the whole app. We should rather check, when this is happening and fix the root cause

AlexAndBear · 2026-02-11T09:08:19Z

packages/web-app-mail/src/components/MailComposeForm.vue

-  modelValue: ComposeFormState
-  showFormattingToolbar?: boolean
-}>()
+const props = withDefaults(


Since you only set showFormattingToolbar to false and not set a complex object, I don't think this is necessary, if not in the consuming component, it should be false either way

AlexAndBear · 2026-02-11T09:13:58Z

packages/web-app-mail/src/components/MailComposeForm.vue

+  }
+)
+
+const { modelValue, showFormattingToolbar } = toRefs(props)


Usually, we don't do this, please check if you can find away around

AlexAndBear · 2026-02-11T09:16:03Z

packages/web-app-mail/src/components/MailComposeForm.vue


 const updateField = <K extends keyof ComposeFormState>(key: K, value: ComposeFormState[K]) => {
-  emit('update:modelValue', { ...modelValue, [key]: value })
+  emit('update:modelValue', { ...modelValue.value, [key]: value })


please use unref, only use .value if you want to set the prop

AlexAndBear · 2026-02-11T09:19:31Z

packages/web-app-mail/src/components/MailList.vue

        </li>
      </oc-list>
-      <MailWidget v-model="showCompose" />
+      <MailWidget v-if="showCompose" v-model="showCompose" />


I think we can kick v-model now

AlexAndBear · 2026-02-11T09:21:56Z

packages/web-app-mail/src/components/MailListItem.vue

 })
+
+const previewText = computed(() => {
+  const p = mail.preview ?? ''


Just check if empty and do eraly return if so, otherwise just jagg it trough the sanitizer

AlexAndBear · 2026-02-11T09:39:55Z

packages/web-app-mail/src/components/MailWidget.vue

            <MailComposeAttachmentButton
              v-model="composeState.attachments"
-              :account-id="composeState.from?.accountId"
+              :account-id="composeState.from?.accountId ?? accountId"


Please use currentAccount only, and make sure that we only show sender emails from the currentAccount, as for now, sending mails from different accounts seems to unstable to me

AlexAndBear · 2026-02-11T09:40:46Z

packages/web-app-mail/src/components/MailWidget.vue

            <MailComposeAttachmentButton
              v-model="composeState.attachments"
-              :account-id="composeState.from?.accountId"
+              :account-id="composeState.from?.accountId ?? accountId"


AlexAndBear · 2026-02-11T09:42:29Z

packages/web-app-mail/src/components/MailWidget.vue

              <oc-icon name="text" fill-type="none" class="text-base text-role-on-surface" />
            </oc-button>
+            <div class="ml-auto flex items-center min-w-0">
+              <div


Maybe we can make use of a reusable component

AlexAndBear · 2026-02-11T09:43:40Z

packages/web-app-mail/src/components/MailWidget.vue

      </div>
    </template>
  </oc-modal>
+  <oc-modal v-if="leaveModalOpen" :title="$gettext('Leave this screen?')" :hide-actions="true">


Please make a new component out of it

AlexAndBear · 2026-02-11T09:44:40Z

packages/web-app-mail/src/components/MailWidget.vue

+
+const { showSavedHint, flashSavedHint, clearSavedHint } = useSavedHint(SAVED_HINT_DURATION_MS)
+
+const accountId = computed(() => unref(currentAccount)?.accountId ?? '')


only use currentAccountId

AlexAndBear · 2026-02-11T09:48:01Z

packages/web-app-mail/src/components/MailWidget.vue

+const draftMailboxId = computed(() => unref(draftsMailboxId) ?? '')
+
+const canSaveDraft = computed(() => {
+  return !!accountId.value && !!draftMailboxId.value


accountId should always be set I don't think we need to check, please use unref

AlexAndBear · 2026-02-11T09:58:29Z

packages/web-app-mail/src/composables/piniaStores/accounts.ts

-    currentAccountIdQuery.value = data?.accountId
-  }
+    const hasAccounts = accounts.value.length > 0
+    const hasValidCurrent = !!currentAccount.value


We don't want to set the currentAccount if we fill the accounts list. If you came across a bug and tried to fix it here. But this is not the right place,
if the bug reoccures please check if there is an issue in the initial loader / setter
in packages/web-app-mail/src/views/Inbox.vue

AlexAndBear · 2026-02-11T09:59:09Z

packages/web-app-mail/src/composables/piniaStores/helpers.ts

@@ -0,0 +1,20 @@
+import { computed, type Ref } from 'vue'


wrong path 🤡

AlexAndBear · 2026-02-11T10:00:35Z

packages/web-app-mail/src/composables/piniaStores/mailboxes.ts

-    if (mailbox) {
-      mailbox[field] = value
+    if (!currentMailboxId.value && mailboxes.value.length) {
+      currentMailboxId.value = mailboxes.value[0].id


We don't want to set the currentMailbox here, if we just fill the list, I think you came accross a bug and tried to fix it here. But this is not the right place,
if the bug reoccures please check if there is an issue in the initial loader / setter
in packages/web-app-mail/src/views/Inbox.vue

AlexAndBear · 2026-02-11T10:01:18Z

packages/web-app-mail/src/composables/piniaStores/mails.ts

-      currentMailId.value = null
-      currentMailIdQuery.value = null
+    if (currentMailId.value && !currentMail.value) {
+      currentMailId.value = ''


We don't want to set the currentMail if we just fill the mails list, I think you came accross a bug and tried to fix it here. But this is not the right place,
if the bug reoccures please check if there is an issue in the initial loader / setter
in packages/web-app-mail/src/views/Inbox.vue

AlexAndBear · 2026-02-11T10:05:48Z

packages/web-app-mail/src/helpers/mailComposeText.ts

+  return (value ?? '').replace(/\s+/g, ' ').trim()
+}
+
+export const plainTextForChangeCheck = (html: string) => {


you don't need this, just use v-html="props.text" in your component

AlexAndBear · 2026-02-11T10:11:45Z

packages/web-app-mail/src/helpers/mailComposeText.ts

+  }
+
+  if (!raw.includes('<')) {
+    return normalizeWhitespace(raw)


We shouldn't replace the user input; If the white spaces bother us in the preview, we should adjust the representation

AlexAndBear · 2026-02-11T10:19:27Z

packages/web-app-mail/src/helpers/mailDraftConnector.ts

+  return segments.map((segment) => encodeURIComponent(segment)).join('/')
+}
+
+export function createMailDraftConnector(http: HttpLike, groupwareUrl: string): MailDraftApi {


Please make a real compososable out of that

We don't need to pass the clientService and configStore props, just use e.G useClientService

Kill the HttpLike interface, it's useless as clientService has it's own interface

Added draft state handling

1bdc3be

tammi-23 added Type:Feature-Request Type:Enhancement and removed Type:Feature-Request labels Feb 9, 2026

tammi-23 requested a review from Copilot February 9, 2026 22:34

Copilot AI reviewed Feb 9, 2026

View reviewed changes

tammi-23 and others added 3 commits February 10, 2026 08:56

wip: only adding name

2cc0413

Apply suggestions from code review

71cbd69

Co-authored-by: Copilot <[email protected]>

wip: added suggestions from review

33654eb

tammi-23 requested a review from Copilot February 10, 2026 13:14

Copilot AI reviewed Feb 10, 2026

View reviewed changes

wip: added suggestions from review2

bbe82a6

tammi-23 requested a review from AlexAndBear February 10, 2026 15:37

tammi-23 marked this pull request as ready for review February 10, 2026 15:38

AlexAndBear reviewed Feb 11, 2026

View reviewed changes


		const { showSavedHint, flashSavedHint, clearSavedHint } = useSavedHint(SAVED_HINT_DURATION_MS)

		const accountId = computed(() => unref(currentAccount)?.accountId ?? '')

Added draft state handling #1956

Are you sure you want to change the base?

Added draft state handling #1956

Uh oh!

Conversation

tammi-23 commented Feb 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants