feat: implement mail account switcher#1677
Conversation
There was a problem hiding this comment.
Pull request overview
This PR replaces the vertical mail account list sidebar with a dropdown-based account switcher integrated into the mailbox tree. The new switcher provides a more compact UI by moving account selection from a dedicated sidebar to a button at the bottom of the mailbox navigation panel.
Key changes:
- Replaced
MailAccountListcomponent with newMailAccountSwitchcomponent - Removed the 100px-wide sidebar from the inbox layout
- Changed several async function signatures to return promises directly instead of being async functions
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/web-app-mail/src/views/Inbox.vue | Removed MailAccountList component and its sidebar container from the inbox view |
| packages/web-app-mail/src/components/MailboxTree.vue | Added MailAccountSwitch component at the bottom of the mailbox tree container |
| packages/web-app-mail/src/components/MailAccountSwitch.vue | New dropdown-based account switcher with avatar, name, and email display |
| packages/web-app-mail/src/components/MailAccountList.vue | Removed the old vertical account list component |
| packages/web-app-mail/src/composables/useLoadMails.ts | Removed async keyword from loadMails function |
| packages/web-app-mail/src/composables/useLoadMailboxes.ts | Removed async keyword from loadMailboxes function |
| packages/web-app-mail/src/composables/useLoadMail.ts | Removed async keyword from loadMail function |
| packages/web-app-mail/src/composables/useLoadAccounts.ts | Removed async keyword from loadAccounts function |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| setCurrentMailbox(unref(mailboxes)[0]) | ||
| await loadMails(unref(currentAccount).accountId, unref(currentMailbox).id) |
There was a problem hiding this comment.
Potential runtime error if mailboxes array is empty after loading. Add a check to ensure the array has at least one element before accessing index 0.
| setCurrentMailbox(unref(mailboxes)[0]) | |
| await loadMails(unref(currentAccount).accountId, unref(currentMailbox).id) | |
| if (unref(mailboxes) && unref(mailboxes).length > 0) { | |
| setCurrentMailbox(unref(mailboxes)[0]) | |
| await loadMails(unref(currentAccount).accountId, unref(mailboxes)[0].id) | |
| } else { | |
| setCurrentMailbox(null) | |
| // Optionally handle the empty mailbox case here (e.g., show a message) | |
| } |
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
| import { AppLoadingSpinner } from '@opencloud-eu/web-pkg' | ||
| import type { MailAccount } from '../types' | ||
| import { useLoadAccounts } from '../composables/useLoadAccounts' | ||
| import { useAccountsStore } from '../composables/piniaStores/accounts' |
There was a problem hiding this comment.
just noticing it here: could you add index.ts files which export all the files of a dir, so that we can merge multiple imports into one line? like we do it in all other apps and packages...
|
Super shiny now, nice job! 😍 |
Description
Related Issue
How Has This Been Tested?
Types of changes