Skip to content

Conversation

@AlexAndBear
Copy link
Contributor

@AlexAndBear AlexAndBear commented Nov 27, 2025

Description

image image

Related Issue

How Has This Been Tested?

  • test environment:
  • test case 1:
  • test case 2:
  • ...

Types of changes

  • Bugfix
  • Enhancement (a change that doesn't break existing code or deployments)
  • Breaking change (a modification that affects current functionality)
  • Technical debt (addressing code that needs refactoring or improvements)
  • Tests (adding or improving tests)
  • Documentation (updates or additions to documentation)
  • Maintenance (like dependency updates or tooling adjustments)

@AlexAndBear AlexAndBear requested a review from kulmann November 27, 2025 13:27
@AlexAndBear AlexAndBear marked this pull request as ready for review November 27, 2025 13:28
Copilot AI review requested due to automatic review settings November 27, 2025 13:28
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 MailAccountList component with new MailAccountSwitch component
  • 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.

Comment on lines +76 to +77
setCurrentMailbox(unref(mailboxes)[0])
await loadMails(unref(currentAccount).accountId, unref(currentMailbox).id)
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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)
}

Copilot uses AI. Check for mistakes.
AlexAndBear and others added 3 commits November 27, 2025 14:31
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
import { AppLoadingSpinner } from '@opencloud-eu/web-pkg'
import type { MailAccount } from '../types'
import { useLoadAccounts } from '../composables/useLoadAccounts'
import { useAccountsStore } from '../composables/piniaStores/accounts'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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...

@kulmann
Copy link
Contributor

kulmann commented Nov 28, 2025

Super shiny now, nice job! 😍

@AlexAndBear AlexAndBear merged commit d634580 into main Nov 28, 2025
28 checks passed
@AlexAndBear AlexAndBear deleted the mail-app-account-switcher branch November 28, 2025 11:56
@openclouders openclouders mentioned this pull request Nov 28, 2025
1 task
@openclouders openclouders mentioned this pull request Dec 15, 2025
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mail: new account switcher

3 participants