Conversation
tammi-23
added
Type:Feature-Request
Type:Enhancement
and removed
Type:Feature-Request
labels
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| account.identities?.map((identity) => ({ | ||
| label: identity.name ? `${identity.name} <${identity.email}>` : identity.email, | ||
| value: identity.id, | ||
| name: identity.name ?? identity.email, |
| <oc-modal v-if="modelValue" :title="$gettext('Leave this screen?')" :hide-actions="true"> | ||
| <template #content> | ||
| <p | ||
| class="oc-mb-m" |
There was a problem hiding this comment.
please use tailwind classes
| ) | ||
| " | ||
| /> | ||
| <div class="oc-modal-body-actions flex justify-end p-4 text-right -mr-4 -mb-4 -ml-4"> |
| const mailPreview = mail.preview ?? '' | ||
| if (!mailPreview) { | ||
| return '' | ||
| } |
There was a problem hiding this comment.
use early return, don't save in extra variable
| <script setup lang="ts"> | ||
| import { useGettext } from 'vue3-gettext' | ||
|
|
||
| defineProps<{ show: boolean }>() |
There was a problem hiding this comment.
Don't use show here, just use v-if or v-show, where the component gets used
| <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"> | ||
| <MailSavedHint :show="showSavedHint" /> |
| ...(hasHtml ? { h: { value: bodyHtml } } : {}) | ||
| }, | ||
|
|
||
| attachments: (state.attachments ?? []).map((a: any) => ({ |
There was a problem hiding this comment.
please write attachement instead of a
| @@ -0,0 +1,78 @@ | |||
| import { watch, onBeforeUnmount, toValue, type MaybeRefOrGetter } from 'vue' | |||
|
|
|||
| export const useAutoSaveDraft = <T>(opts: { | |||
There was a problem hiding this comment.
please destructure the object and don't use opts
| @@ -0,0 +1,13 @@ | |||
| import { computed } from 'vue' | |||
There was a problem hiding this comment.
I don't think we need this composable as of now.
Please use queryItemAsStringand useRouteQuery.value = id
if we need this, please move it to web-pkg as it's a general helper/composable
| const isDirty = ref(false) | ||
|
|
||
| const canSave = computed(() => { | ||
| return !!toValue(opts.accountId) && !!toValue(opts.draftMailboxId) |
There was a problem hiding this comment.
Please chek if we need to value, this is making the code more complex and we should have a clear interface. At least 'accountId' is not a getter so MaybeRefOrGetter is wrong
| const value = input ?? '' | ||
| if (!value) { | ||
| return '' | ||
| } |
2 participants
implement #1480