Skip to content

feat: add filter-based mass selection via search modal#12901

Open
RobinAngele wants to merge 46 commits into
nextcloud:mainfrom
RobinAngele:feat/filter-mass-select
Open

feat: add filter-based mass selection via search modal#12901
RobinAngele wants to merge 46 commits into
nextcloud:mainfrom
RobinAngele:feat/filter-mass-select

Conversation

@RobinAngele
Copy link
Copy Markdown

@RobinAngele RobinAngele commented May 7, 2026

Depends on: #12900

Closes #4285
Closes #7880
Closes #11527
Refs #6070
Refs #7276
Refs #11526
Refs #11973
Refs #11420

Summary

Built on #12900 which centralised selection state in Mailbox, this PR completes #4285 and #7880 by adding three complementary ways to select messages for bulk actions:

  1. Checkbox row — click the checkbox at the top of the list to select all currently loaded messages at once.

  2. Select-all banner — after clicking the checkbox to select all loaded messages, if more pages remain a banner appears. Clicking it loads all remaining pages and selects every message in the current view.

  3. Search modal "Select all matching" (new primary button) — open the search dialog, set any filter combination, and click "Select all matching". The modal closes, all matching messages are loaded across all pages, and the entire result set is selected ready for bulk action.

In all three flows, selection is capped at 500 messages to prevent OOM in unified mailboxes (which fan out to N IMAP accounts per page fetch). When the cap is reached a warning explains it and suggests narrowing the filter or working in batches.

Also fixes shift-click range selection across date groups (#11527): shift-clicking a message in a different date group from the last-selected one now correctly selects all messages in between.

Also adds full Priority inbox support: each of the four sections (Favorites, Follow-up, Important, Other) gets its own independent select-all flow. Selecting messages in one section automatically clears the selection in any other, since each section has its own action toolbar and cross-section bulk operations are not possible.

Screenshots

Search modal — new "Select all matching" primary button

Screenshot_20260507_114235_select_feat_Search parameters_ovelay

Mass-select result — N selected with action toolbar

Screenshot_20260510_102704_selft_filter_with_result_background

Priority inbox mutual exclusion — section cleared toast

Screenshot_20260510_103559_toast_priority_mailbox

Architecture

Component interaction

User clicks "Select all matching" in search modal
        │
        ▼
SearchMessages.vue
  emits 'select-all-matching' (query string)
        │
        ▼
MailboxThread.vue
  sets this.searchQuery = query          ← must happen before bus emit
  emits bus('select-all-matching')       ← broadcast to all Mailbox instances
        │
        ├─────────────────────────────────────────────┐
        ▼                                             ▼
Mailbox.vue × 2 (regular / Unified inbox) Mailbox.vue × 4 (Priority inbox only)
  Favorites  (isPriorityInbox=true)     Favorites, Follow-up, Important, Other
  Main       (isPriorityInbox=false)    all have isPriorityInbox=true
  share one mitt bus                    share one mitt bus
  Main owns the select-all banner       each fetches its own filtered pages
  each fetches its own filtered pages   mutual exclusion via 'section-selected'
        │                                             │
        ▼                                             ▼
EnvelopeList.vue (per section)          EnvelopeList.vue (per section)
  receives selection[] as prop            receives selection[] as prop
  emits update:selection (delta)          emits update:selection / select-range
  emits select-range (flat indices)

State flows down via props, changes flow up via Vue events, and cross-instance coordination uses the shared mitt bus.


Key design decisions

Every MailboxThread view mounts at least two Mailbox instances on the same bus — a Favorites section and a Main section. The Priority inbox (PRIORITY_INBOX_ID) extends this to four sections (Favorites, Follow-up, Important, Other), all with isPriorityInbox=true. The unified inbox (UNIFIED_INBOX_ID) uses the same two-section layout as a regular folder and requires no special handling. The decisions below address the non-obvious coordination problems the multi-instance shared-bus pattern creates.

1. Capturing the settled query before loading
Priority inbox sections inject a query suffix one Vue render tick after the parent's searchQuery prop changes. Starting mass-select immediately would use a stale query and corrupt the cache key. The fix is to raise a lock flag (loadingAllMatching) before yielding — which simultaneously blocks the searchQuery watcher from triggering a competing reload — then read the settled, section-filtered query after the tick.

2. Mutual exclusion between sections
Each section has its own independent action toolbar, so a selection spanning two sections can't be acted upon in a single bulk operation. When a section's selection goes from empty to non-empty (user-initiated), it broadcasts a section-selected event on the shared bus. Any sibling holding a selection clears itself with an info toast.

3. Concurrent mass-selects must not interfere
The search modal triggers mass-select on all sections simultaneously — each independently loads its own filtered pages. Mutual exclusion must not fire on completion: each finishing section would otherwise clear the others' just-loaded selections. So section-selected is emitted only on user-initiated transitions, never at the end of a programmatic mass-load.

4. OOM protection: 500-message cap
The unified inbox fans out to N IMAP accounts per page fetch. Without a ceiling, large multi-account selections exhaust client memory. The loop halts at 500 and shows a visible warning rather than silently truncating.

5. Keeping the spinner visible during mass-select
Mass-select forces a cache invalidation to guarantee a fresh fetch. Without a guard, this triggers LoadingSkeleton, hiding the spinner mid-operation. A !loadingAllMatching condition in loadEnvelopes() suppresses the skeleton so the spinner stays visible throughout.

Changes

SearchMessages.vue

  • Added "Select all matching" as a primary NcDialog button; "Search" demoted to tertiary
  • selectAllMatching() sets match = 'allof', closes the modal, then emits select-all-matching with the full computed query string — intentionally no sendQueryEvent() to avoid a concurrent reload
  • closeSearchModal() has an idempotency guard — safe to call multiple times
  • Fixed filterData.mentions: coerces false to '' so mentions:false is never appended to the IMAP query string

MailboxThread.vue

  • Wires @select-all-matching="onSelectAllMatching" on SearchMessages
  • onSelectAllMatching(query) sets this.searchQuery = query before this.bus.emit('select-all-matching') so each Priority inbox section reads the correct parent query when it captures effectiveQuery after its own $nextTick

Mailbox.vue

  • isPriorityInbox prop — hides the select-all banner, disables envelope grouping, and shows an empty-section placeholder (mutual exclusion via section-selected fires on all instances regardless of this flag)
  • loadingAllMatching / selectAllMatching / selectionLimitReached data flags
  • onBusSelectAllMatching() — full mass-select lifecycle: raises flag, yields tick, clears cache, fetches all pages, caps at 500, selects all; both failure paths (page-load error mid-loop and outer catch) now show an error toast and reset selectAllMatching (previously silent — console-only logging)
  • loadEnvelopes()!loadingAllMatching guard suppresses LoadingSkeleton during mass-select
  • selection watcher — emits section-selected on 0 → non-zero transition, guarded by !loadingAllMatching
  • onBusSectionSelected(activeQuery) — clears sibling section selection with info toast ("Selection cleared — only one Priority inbox section can be selected at a time"); self-guard via activeQuery !== this.searchQuery
  • selectAllMatchingAction() — in-page banner entry point (calls onBusSelectAllMatching() directly, no bus round-trip)
  • selectAllLabel computed — context-aware label:
Condition Label
loadingAllMatching Selecting messages…
selectMode N selected
hasFilter Select N matching messages
!endReached && count > 0 Select N loaded messages
(default) Select all N messages
  • select-all-banner — prompts to load all matching when all visible are selected and more exist; hidden for Priority inbox; button label adapts: "Select all matching messages" when a filter is active, "Select all messages in this folder" otherwise
  • select-all-limit-warning — shown in warning colour when 500-cap is hit: "Selection limited to N messages — use a filter to narrow results, or process in several batches."
  • onUpdateSelection() — merged into single array assignment (removes intermediate empty reactive state)
  • unselectAll() — also clears loadingAllMatching and expanded
  • bus.on/off('section-selected', ...) symmetric in created/unmounted
  • Imports priorityImportantQuery, priorityOtherQuery, and favoritesQuery constants instead of hardcoding strings in hasFilter

EnvelopeList.vue

  • _localToggleInProgress guard on both selection and sortedEnvelops watchers — prevents watchers from overwriting flags.selected during a local click; reset via $nextTick to survive Vue's watcher flush

How to test

  • Open any folder → click the checkbox row → all visible messages selected, action toolbar appears
  • Open a large folder (more than one page of messages) → scroll down to load a second page → without selecting anything, verify the checkbox-row label updates to "Select N loaded messages" where N reflects only the messages loaded so far, not the full folder total
  • Select all visible in a large folder (no filter) → banner appears → click it → all pages load and select
  • Open the search modal → set a filter → click "Search" (tertiary button, not "Select all matching") → filter results load → select all loaded via checkbox → banner appears with filter wording → click it → all matching messages across all pages selected
  • Open the search modal → set any filter → click "Select all matching" (primary button) → modal closes, spinner appears, all matching messages across all pages are selected directly (no banner step)
  • While messages are selected, click individual avatar circles to deselect/reselect one at a time → count updates correctly, no spurious multi-deselect
  • Shift-click across messages in different date groups → range selected correctly
  • Priority inbox → open Priority inbox → click avatar circles in the Important section to select one or more messages → then click an avatar circle in the Other section → verify the Important selection clears automatically and a toast appears: "Selection cleared — only one Priority inbox section can be selected at a time"
  • Priority inbox → select in Important → click individual avatars to add/remove messages within the same section → sibling sections are not affected
  • Open a folder with > 500 messages → trigger mass-select via the banner or search modal → selection stops at 500 and shows the cap warning in orange with actionable advice
  • Open DevTools → Network tab → start a mass-select on a large folder → set network to Offline mid-load → spinner disappears, error toast appears, no messages remain selected
  • Bulk actions (delete, mark read, move, tag) apply correctly to all selected messages

Known limitations

  • No cross-section bulk actions in Priority inbox. Selecting in Important and then clicking in Other clears the first section. There is no toolbar that spans sections, so a cross-section selection cannot be acted upon.
  • 500-message cap. Raising it requires backend pagination performance improvements first.
  • No server-side "select all" flag. Bulk operations iterate over the selection array and fire individual API calls. A future improvement would be a server-side endpoint accepting a filter query and performing the operation without enumerating IDs client-side

…ctions

- Add missing ImportantIcon import and component registration in
  EnvelopeList so the 'Mark as important' icon renders during
  bulk selection
- Fix favorite/unfavorite bulk action logic: rename methods to
  favoriteAll/unfavoriteAll and use explicit favFlag values
  (true/false) instead of inverted computed checks that failed
  when all selected messages shared the same state

AI-assisted: Cline (Claude)
Signed-off-by: RobinAngele <git@robin4consulting.com>
Add a 'Select all X messages' checkbox above the envelope list using
NcCheckboxRadioSwitch from @nextcloud/vue, allowing users to select
all visible messages at once.

Lift envelope selection state from individual EnvelopeList instances
up to the Mailbox parent component. This enables:
- Cross-group shift-click range selection via flat envelope indexing
- Consistent selection state across grouped envelope lists
- Global select-all / unselect-all from the parent level

Also pass the selection array as a prop to EnvelopeList and add
proper event handling for update:selection and select-range.

Closes: nextcloud#4285
Refs: nextcloud#7880, nextcloud#6070, nextcloud#7276

AI-assisted: Cline (Claude)
Signed-off-by: RobinAngele <git@robin4consulting.com>
Add a 'Select all matching' button to the search parameters dialog
and enable selecting all messages matching a filter across all pages.

Search modal:
- New 'Select all matching' button in SearchMessages.vue dialog
- Emits 'select-all-matching' event via the MailboxThread bus

Mass loading:
- Mailbox.vue onBusSelectAllMatching forces a fresh load, then
  iterates loadMore() until all pages are fetched
- Spinner + 'Selecting messages…' shown during loading
- Checkbox disabled while loading

Context-aware labels:
- 'Select N loaded messages' when more pages exist
- 'Select N matching messages' when a filter is active
- 'Select all N messages' when all pages are loaded
- Hint row: 'Scroll down to include more messages, use filter to
  refine, or click an avatar circle to select one at a time'

Depends on: nextcloud#12900 (select-all checkbox feature)
Closes: nextcloud#4285, nextcloud#7880
Refs: nextcloud#6070, nextcloud#7276, nextcloud#11526

AI-assisted: Cline (Claude)
Signed-off-by: RobinAngele <git@robin4consulting.com>
Use selectMode (any selection) instead of allSelected (must match all
visible) for the checkbox checked state. This way the checkbox stays
checked when the user manually deselects individual messages via
avatar circles after using Select All.

AI-assisted: Cline (Claude)
Signed-off-by: RobinAngele <git@robin4consulting.com>
…s active

When a filter is already applied, the hint now shows:
'Scroll down to include more messages or click an avatar circle
to select one at a time' — removing the redundant filter advice.

AI-assisted: Cline (Claude)
Signed-off-by: RobinAngele <git@robin4consulting.com>
When in selectMode, the checkbox label now shows '{N} selected'
with the real count instead of the loaded count. This way when
the user deselects individual messages, the number updates
immediately from 20 to 19, etc.

AI-assisted: Cline (Claude)
Signed-off-by: RobinAngele <git@robin4consulting.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 8, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR lifts selection into Mailbox, adds a select-all UI and cross-page “select all matching” flow, converts EnvelopeList to prop-driven selection with flatIndex for global ranges, delegates range selection to Mailbox, updates favorite/unfavorite bulk actions, wires SearchMessages and MailboxThread to emit selection events, refactors envelope shift-click handling, and adds documentation and UI styles.

Suggested reviewers

  • GretaD
  • kesselb
  • ChristophWurst
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main feature added: filter-based mass selection initiated from the search modal.
Linked Issues check ✅ Passed The PR fully implements the requirements from #4285 and #7880: selecting all messages in a mailbox and providing a select-all action for bulk operations.
Out of Scope Changes check ✅ Passed All changes are directly scoped to implementing filter-based mass selection: SearchMessages dialog, Mailbox selection state, EnvelopeList multi-select support, and supporting infrastructure.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/components/Mailbox.vue (1)

317-335: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Reset pagination and bulk-selection state when the source list changes.

These watchers clear selection, but they leave endReached / expanded behind and only partially reset the select-all-matching flags. If the previous view had already reached the end, the next mailbox/filter/sort starts in a stale “fully loaded” state, which can hide further pages and prevent the new result set from using the intended bulk-select flow.

Suggested reset
 		mailbox() {
 			this.selection = []
+			this.selectAllMatching = false
+			this.loadingAllMatching = false
+			this.endReached = false
+			this.expanded = false
 			this.loadEnvelopes()
 				.then(() => {
 					logger.debug(`syncing mailbox ${this.mailbox.databaseId} (${this.query}) after folder change`)
 					this.sync(false)
 				})
 		},

 		searchQuery() {
 			this.selection = []
 			this.selectAllMatching = false
 			this.loadingAllMatching = false
+			this.endReached = false
+			this.expanded = false
 			this.loadEnvelopes()
 		},

 		sortOrder() {
 			this.selection = []
+			this.selectAllMatching = false
+			this.loadingAllMatching = false
+			this.endReached = false
+			this.expanded = false
 			this.loadEnvelopes()
 		},
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/Mailbox.vue` around lines 317 - 335, The watchers mailbox(),
searchQuery(), and sortOrder() currently clear selection but leave pagination
and bulk-selection state stale; update each watcher to also reset endReached
(set to false), clear expanded state (reset the expanded map/array), and set
selectAllMatching and loadingAllMatching to false before calling loadEnvelopes()
(and before sync in mailbox()), so the new mailbox/filter/sort starts with fresh
pagination and bulk-selection state; reference the mailbox(), searchQuery(), and
sortOrder() watcher handlers and the component properties endReached, expanded,
selectAllMatching, loadingAllMatching, selection, loadEnvelopes(), and sync().
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/components/MailboxThread.vue`:
- Around line 607-609: The onSelectAllMatching handler currently emits a generic
'select-all-matching' on the shared mitt bus causing every Mailbox subscriber to
react; change it to emit a scoped payload (e.g., include mailboxId or query) or
target the specific Mailbox directly. Update the onSelectAllMatching method to
call this.bus.emit('select-all-matching', { mailboxId: this.mailboxId, query:
this.currentQuery }) (or invoke a direct method on the intended Mailbox
instance) and update Mailbox subscribers to check the payload (e.g., mailboxId
or query) before starting load/select so only the intended mailbox responds.
Ensure the event name remains 'select-all-matching' but listeners in Mailbox use
the identifying field to ignore unrelated events.

In `@src/components/SearchMessages.vue`:
- Around line 578-584: The selectAllMatching() method currently calls
sendQueryEvent() and emits 'select-all-matching' inside a single
this.$nextTick(), which can let Mailbox receive a stale
MailboxThread.searchQuery prop; change the flow in selectAllMatching to call
this.sendQueryEvent() inside the first this.$nextTick(), then call a second
this.$nextTick() before emitting this.$emit('select-all-matching') so
Mailbox.onBusSelectAllMatching() receives the updated prop value and avoids
corrupting the cache key logic.

---

Outside diff comments:
In `@src/components/Mailbox.vue`:
- Around line 317-335: The watchers mailbox(), searchQuery(), and sortOrder()
currently clear selection but leave pagination and bulk-selection state stale;
update each watcher to also reset endReached (set to false), clear expanded
state (reset the expanded map/array), and set selectAllMatching and
loadingAllMatching to false before calling loadEnvelopes() (and before sync in
mailbox()), so the new mailbox/filter/sort starts with fresh pagination and
bulk-selection state; reference the mailbox(), searchQuery(), and sortOrder()
watcher handlers and the component properties endReached, expanded,
selectAllMatching, loadingAllMatching, selection, loadEnvelopes(), and sync().
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 495a2697-7608-4c97-89cd-216e17ceac3f

📥 Commits

Reviewing files that changed from the base of the PR and between 217ec4d and cf302c5.

📒 Files selected for processing (4)
  • src/components/EnvelopeList.vue
  • src/components/Mailbox.vue
  • src/components/MailboxThread.vue
  • src/components/SearchMessages.vue

Comment thread src/components/MailboxThread.vue Outdated
Comment thread src/components/SearchMessages.vue Outdated
Added _localToggleInProgress guard that prevents the selection
watcher from overwriting flags.selected during a local click
toggle. This fixes a bug where clicking one avatar to deselect
would sometimes remove multiple messages at once.

AI-assisted: Cline (Claude)
Signed-off-by: RobinAngele <git@robin4consulting.com>
When loadingAllMatching is true, selectAllLabel now returns
'Selecting messages…' instead of the contextual label
(matching/loaded/all). This prevents showing stale filter
labels while loading is in progress.

AI-assisted: Cline (Claude)
Signed-off-by: RobinAngele <git@robin4consulting.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
src/components/Mailbox.vue (2)

830-846: ⚖️ Poor tradeoff

Consider adding a cancellation mechanism for long-running operations.

The while (!this.endReached) loop could run for a very long time on large mailboxes, especially over slow connections. If the user changes the filter, navigates away, or the component unmounts during loading, the loop continues until completion.

Consider:

  1. Adding an abort flag checked in the loop
  2. Implementing a page limit (e.g., max 50 pages)
  3. Checking component mount state before state updates
♻️ Example cancellation pattern
 async selectAllMatchingAction() {
     this.loadingAllMatching = true
     this.selectAllMatching = true
+    this._selectAllAborted = false

     try {
         // Load remaining pages until all envelopes are fetched
-        while (!this.endReached) {
+        while (!this.endReached && !this._selectAllAborted) {
             await this.loadMore()
         }
+        if (this._selectAllAborted) {
+            return
+        }
         // Now select all loaded envelopes
         this.selection = this.flatEnvelopeList.map((e) => e.databaseId)
     } catch (error) {
         logger.error('Failed to load all matching envelopes', { error })
     } finally {
         this.loadingAllMatching = false
     }
 },

Then set this._selectAllAborted = true in unselectAll() and watchers that reset state.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/Mailbox.vue` around lines 830 - 846, The
selectAllMatchingAction can run indefinitely; add a cancellable pattern:
introduce a boolean flag (e.g. this._selectAllAborted) and a maxPage cap (e.g.
const MAX_SELECT_PAGES = 50) and modify selectAllMatchingAction to check both
the abort flag and page cap inside the while (!this.endReached) loop before each
await this.loadMore() and break if triggered; ensure you check the abort flag
before assigning this.selection or calling this.flatEnvelopeList, and clear
loadingAllMatching on abort; set this._selectAllAborted = true from
unselectAll(), relevant filter-change watchers, and the component
beforeDestroy/unmounted hook so the in-flight loop exits and avoids updating
state after unmount.

36-48: 💤 Low value

Dead code: loading icon inside the banner will never render.

The banner's v-if condition includes !selectAllMatching, but loadingAllMatching and selectAllMatching are always set together (both in selectAllMatchingAction and onBusSelectAllMatching). When loadingAllMatching is true, selectAllMatching is also true, so the banner won't render and the NcLoadingIcon on line 39 is unreachable.

Consider removing the loading icon from the banner or adjusting the banner's visibility condition if loading state should be shown here.

♻️ Suggested fix
 <div
     v-if="allSelected && !selectAllMatching && flatEnvelopeList.length < totalEnvelopeCount"
     class="select-all-banner">
-    <NcLoadingIcon v-if="loadingAllMatching" :size="16" />
     <span>{{ n('mail',
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/Mailbox.vue` around lines 36 - 48, The banner's v-if
(allSelected && !selectAllMatching && flatEnvelopeList.length <
totalEnvelopeCount) prevents loadingAllMatching from ever being visible because
selectAllMatching is set alongside loadingAllMatching in selectAllMatchingAction
and onBusSelectAllMatching; either remove the unreachable NcLoadingIcon or
change the banner condition to allow the loading state — e.g. update the v-if to
include loadingAllMatching (allSelected && (loadingAllMatching ||
!selectAllMatching) && flatEnvelopeList.length < totalEnvelopeCount) so
NcLoadingIcon can render while select-all is in progress, or simply delete the
NcLoadingIcon element if you prefer not to show loading there.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/components/Mailbox.vue`:
- Around line 21-28: The checkbox is listening to the wrong event: change the
event handler on the NcCheckboxRadioSwitch from `@update`:checked to
`@update`:model-value so toggles actually invoke your handlers; update the
component invocation that references selectMode and the methods selectAll() /
unselectAll() to use `@update`:model-value="selectMode ? unselectAll() :
selectAll()" so the v-model update triggers those functions.

---

Nitpick comments:
In `@src/components/Mailbox.vue`:
- Around line 830-846: The selectAllMatchingAction can run indefinitely; add a
cancellable pattern: introduce a boolean flag (e.g. this._selectAllAborted) and
a maxPage cap (e.g. const MAX_SELECT_PAGES = 50) and modify
selectAllMatchingAction to check both the abort flag and page cap inside the
while (!this.endReached) loop before each await this.loadMore() and break if
triggered; ensure you check the abort flag before assigning this.selection or
calling this.flatEnvelopeList, and clear loadingAllMatching on abort; set
this._selectAllAborted = true from unselectAll(), relevant filter-change
watchers, and the component beforeDestroy/unmounted hook so the in-flight loop
exits and avoids updating state after unmount.
- Around line 36-48: The banner's v-if (allSelected && !selectAllMatching &&
flatEnvelopeList.length < totalEnvelopeCount) prevents loadingAllMatching from
ever being visible because selectAllMatching is set alongside loadingAllMatching
in selectAllMatchingAction and onBusSelectAllMatching; either remove the
unreachable NcLoadingIcon or change the banner condition to allow the loading
state — e.g. update the v-if to include loadingAllMatching (allSelected &&
(loadingAllMatching || !selectAllMatching) && flatEnvelopeList.length <
totalEnvelopeCount) so NcLoadingIcon can render while select-all is in progress,
or simply delete the NcLoadingIcon element if you prefer not to show loading
there.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e480ccd4-a6cf-4ccc-9a40-65b3f878b3f0

📥 Commits

Reviewing files that changed from the base of the PR and between cf302c5 and 43e3fb3.

📒 Files selected for processing (2)
  • src/components/EnvelopeList.vue
  • src/components/Mailbox.vue

Comment thread src/components/Mailbox.vue
Vue 2 batches watchers at end of tick. The previous synchronous
reset meant the watcher would fire after _localToggleInProgress
was already false, re-introducing the race condition.

Now reset via $nextTick so the guard remains active during
the watcher's flush.

AI-assisted: Cline (Claude)
Signed-off-by: RobinAngele <git@robin4consulting.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/components/EnvelopeList.vue (1)

393-404: 💤 Low value

Consider removing redundant cleanup code.

The new cleanup logic (lines 395-399) correctly removes vanished envelope IDs from selection and emits the cleaned array. The older differenceWith cleanup (lines 400-403) sets flags.selected = false on envelopes that are no longer in sortedEnvelops, which has no practical effect since those envelopes are already removed from the rendered list. This code could be removed for clarity.

♻️ Suggested cleanup
 		sortedEnvelops(newVal, oldVal) {
 			// Unselect vanished envelopes by emitting cleaned selection
 			const newIds = new Set(newVal.map((env) => env.databaseId))
 			const cleanedSelection = this.selection.filter((id) => newIds.has(id))
 			if (cleanedSelection.length !== this.selection.length) {
 				this.$emit('update:selection', cleanedSelection, this.envelopes)
 			}
-			differenceWith((a, b) => a.databaseId === b.databaseId, oldVal, newVal)
-				.forEach((env) => {
-					env.flags.selected = false
-				})
 		},
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/EnvelopeList.vue` around lines 393 - 404, Remove the redundant
cleanup in the sortedEnvelops watcher: the initial block that builds newIds,
computes cleanedSelection from this.selection and emits update:selection already
cleans vanished IDs, so delete the subsequent differenceWith(...).forEach(env =>
env.flags.selected = false) code path (referenced as differenceWith in the
sortedEnvelops watcher which manipulates env.flags.selected) to avoid
dead/unused state changes while keeping the selection cleanup and
this.$emit('update:selection', cleanedSelection, this.envelopes) intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/components/EnvelopeList.vue`:
- Around line 393-404: Remove the redundant cleanup in the sortedEnvelops
watcher: the initial block that builds newIds, computes cleanedSelection from
this.selection and emits update:selection already cleans vanished IDs, so delete
the subsequent differenceWith(...).forEach(env => env.flags.selected = false)
code path (referenced as differenceWith in the sortedEnvelops watcher which
manipulates env.flags.selected) to avoid dead/unused state changes while keeping
the selection cleanup and this.$emit('update:selection', cleanedSelection,
this.envelopes) intact.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 32a554d8-9c2d-4cd5-bfdc-7a6234f76eed

📥 Commits

Reviewing files that changed from the base of the PR and between 43e3fb3 and 39d0247.

📒 Files selected for processing (1)
  • src/components/EnvelopeList.vue

Added event.shiftKey check in onSelectMultiple to prevent
range selection from firing on plain clicks. Also passes
$event from template to the handler.

AI-assisted: Cline (Claude)
Signed-off-by: RobinAngele <git@robin4consulting.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/components/Envelope.vue (1)

1-4: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Update SPDX header to the required repository format.

Line 2 and Line 3 do not match the mandatory SPDX strings for Vue files in this repository (year/text and identifier key).

🔧 Proposed fix
 <!--
-  - SPDX-FileCopyrightText: 2018 Nextcloud GmbH and Nextcloud contributors
-  - SPDX-License-Identifier: AGPL-3.0-or-later
+  - SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+  - SPDX-Identifier: AGPL-3.0-or-later
 -->

As per coding guidelines "{lib,src,tests}/**/*.{php,js,vue}: Every file must include an SPDX license header" and "SPDX license header format must be: SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors and SPDX-Identifier: AGPL-3.0-or-later".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/Envelope.vue` around lines 1 - 4, Update the SPDX header in
Envelope.vue to the repository-mandated format by replacing the existing two
header lines with the exact required strings: "SPDX-FileCopyrightText: 2026
Nextcloud GmbH and Nextcloud contributors" and "SPDX-Identifier:
AGPL-3.0-or-later"; locate the existing header comment at the top of
src/components/Envelope.vue and swap those two lines so the file matches the
project's SPDX header rules.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/components/Envelope.vue`:
- Around line 1-4: Update the SPDX header in Envelope.vue to the
repository-mandated format by replacing the existing two header lines with the
exact required strings: "SPDX-FileCopyrightText: 2026 Nextcloud GmbH and
Nextcloud contributors" and "SPDX-Identifier: AGPL-3.0-or-later"; locate the
existing header comment at the top of src/components/Envelope.vue and swap those
two lines so the file matches the project's SPDX header rules.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f1de2789-e604-4606-890e-aa1cb989082a

📥 Commits

Reviewing files that changed from the base of the PR and between 39d0247 and 2453c01.

📒 Files selected for processing (1)
  • src/components/Envelope.vue

…parison

Changed hasFilter check from !== 'match:allof' to .length > 12.
This handles cases where closeSearchModal emits a stale query
with residual whitespace or parameters. The default match:allof
query is exactly 12 chars; anything longer has a real filter.

AI-assisted: Cline (Claude)
Signed-off-by: RobinAngele <git@robin4consulting.com>
@RobinAngele RobinAngele force-pushed the feat/filter-mass-select branch from d96ac21 to c5a8cfd Compare May 8, 2026 08:33
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/components/Mailbox.vue (1)

323-341: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Reset pagination and mass-selection flags when the result set changes.

These watchers clear selection, but they leave endReached/expanded behind, and mailbox/sortOrder also leave the bulk-selection flags behind. After fully loading one folder or filter, the next one can start with endReached = true, which hides pagination and makes the new select-all flow stop early.

Suggested fix
 watch: {
 	mailbox() {
-		this.selection = []
+		this.unselectAll()
+		this.endReached = false
+		this.expanded = false
 		this.loadEnvelopes()
 			.then(() => {
 				logger.debug(`syncing mailbox ${this.mailbox.databaseId} (${this.query}) after folder change`)
 				this.sync(false)
 			})
 	},

 	searchQuery() {
-		this.selection = []
-		this.selectAllMatching = false
-		this.loadingAllMatching = false
+		this.unselectAll()
+		this.endReached = false
+		this.expanded = false
 		this.loadEnvelopes()
 	},

 	sortOrder() {
-		this.selection = []
+		this.unselectAll()
+		this.endReached = false
+		this.expanded = false
 		this.loadEnvelopes()
 	},
 },
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/Mailbox.vue` around lines 323 - 341, The mailbox(),
searchQuery(), and sortOrder() watchers currently only clear selection but must
also reset pagination and mass-selection flags to avoid carrying state into the
next result set; update each watcher (mailbox(), searchQuery(), sortOrder()) to
set endReached = false and expanded = false and also clear selectAllMatching =
false and loadingAllMatching = false before calling loadEnvelopes()/sync(),
ensuring any pagination cursor or offset state used by loadEnvelopes() is
effectively reset for the new folder/filter.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/components/Mailbox.vue`:
- Around line 37-47: The banner/button currently uses totalEnvelopeCount as if
it were the full match total even when endReached is false; update the template
logic around the banner (the v-if using allSelected, selectAllMatching and
flatEnvelopeList.length < totalEnvelopeCount) and the copy/CTA (the span,
NcButton and their use of totalEnvelopeCount) so that exact "all matching"
counts are only shown when endReached === true—otherwise show a non-exact CTA
like "Select all visible messages" or "Select all matching messages" (no exact
number) and, if desired, show a progressive "Select all {visible} on this page"
with a secondary loader state (loadingAllMatching) that then resolves to the
exact count when endReached becomes true; apply the same change to the second
identical block that uses the same variables and selectAllMatchingAction.
- Around line 831-846: The selectAllMatchingAction loop can spin indefinitely
when loadMore() throws because endReached never flips; update
selectAllMatchingAction so that if loadMore() rejects you break the while loop
(or set endReached=true) before rethrowing or logging. Specifically, inside
selectAllMatchingAction around the while (!this.endReached) { await
this.loadMore() } call handle errors from loadMore() (or wrap each await
this.loadMore() in its own try/catch) and on error log via logger.error and then
break the loop so the method can proceed to set this.selection from
this.flatEnvelopeList and finish (preserve the existing finally that clears
this.loadingAllMatching and leave this.selectAllMatching true/false behavior
unchanged).

---

Outside diff comments:
In `@src/components/Mailbox.vue`:
- Around line 323-341: The mailbox(), searchQuery(), and sortOrder() watchers
currently only clear selection but must also reset pagination and mass-selection
flags to avoid carrying state into the next result set; update each watcher
(mailbox(), searchQuery(), sortOrder()) to set endReached = false and expanded =
false and also clear selectAllMatching = false and loadingAllMatching = false
before calling loadEnvelopes()/sync(), ensuring any pagination cursor or offset
state used by loadEnvelopes() is effectively reset for the new folder/filter.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7a525e38-36d4-4e8e-8869-8d36f335f18c

📥 Commits

Reviewing files that changed from the base of the PR and between 2453c01 and d96ac21.

📒 Files selected for processing (1)
  • src/components/Mailbox.vue

Comment thread src/components/Mailbox.vue Outdated
Comment thread src/components/Mailbox.vue Outdated
@ChristophWurst
Copy link
Copy Markdown
Member

Sorry if the review rabbit caused any confusion. We enabled the integration but the config to turn auto reviews off was missing.

After a filter loads all pages, endReached stays true. When the
filter is cleared, loadEnvelopes() is called but doesn't reset
endReached, causing the label to skip the 'loaded' state and
show the default 'Select all N messages' instead.

Now the searchQuery watcher resets endReached before reloading.

AI-assisted: Cline (Claude)
Signed-off-by: RobinAngele <git@robin4consulting.com>
The previous .length > 12 check could fail if the default
match:allof query had extra whitespace or encoding artifacts.
Now uses /^match:allof\s*$/ regex for robust detection.

AI-assisted: Cline (Claude)
Signed-off-by: RobinAngele <git@robin4consulting.com>
mentionsMe is a boolean. After resetFilter(), it's false.
filterData returned the raw boolean, and searchQuery builder
added 'mentions:false' to the query because it only skipped
empty strings and null, not false.

Now returns empty string when mentionsMe is false.

AI-assisted: Cline (Claude)
Signed-off-by: RobinAngele <git@robin4consulting.com>
Previously only loadingAllMatching prevented stale labels.
Now loadingEnvelopes (normal filter/mailbox change) also
shows 'Loading…' and disables the checkbox, preventing
stale counts from appearing during async loads.

AI-assisted: Cline (Claude)
Signed-off-by: RobinAngele <git@robin4consulting.com>
The sortedEnvelops watcher could fire during a local toggle
and clear flags.selected on active envelopes via differenceWith.
Added _localToggleInProgress guard to prevent this.

AI-assisted: Cline (Claude)
Signed-off-by: RobinAngele <git@robin4consulting.com>
Normal envelope loading now only changes the label to 'Loading…'
without disabling the checkbox or showing the spinner row.
This prevents the stuck spinner when rapid filter changes cause
overlapping loadEnvelopes calls.

AI-assisted: Cline (Claude)
Signed-off-by: RobinAngele <git@robin4consulting.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/components/Mailbox.vue`:
- Around line 903-913: The mass-select loop currently breaks on a failed
loadMore(effectiveQuery) but still proceeds to set this.selectAllMatching and
this.selection from this.flatEnvelopeList, which can silently produce a partial
selection and exceed MAX_SELECT_MESSAGES; change the flow so that if
loadMore(...) returns false you immediately clear/disable select-all mode (set
this.selectAllMatching = false), do not populate this.selection from partial
results, and set this.selectionLimitReached = true; additionally, after the
fetch loop always enforce a strict cap by truncating any assignment to
this.selection (e.g., use the first MAX_SELECT_MESSAGES of
this.flatEnvelopeList) and log the cap, and apply the same guard in the later
block that sets selection (the block around lines 916-919) so both places
consistently abort select-all on failed load and enforce the hard
MAX_SELECT_MESSAGES cap.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3db28979-8b9d-4a6a-a7bd-2896b8c76045

📥 Commits

Reviewing files that changed from the base of the PR and between d53d66e and 3ff0e5c.

📒 Files selected for processing (2)
  • Nextcloud_Mail_Select_All_and_Mass_Selection.md
  • src/components/Mailbox.vue

Comment thread src/components/Mailbox.vue Outdated
@RobinAngele
Copy link
Copy Markdown
Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 10, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

RobinAngele and others added 2 commits May 10, 2026 11:52
- flatEnvelopeList now mirrors EnvelopeList sortedEnvelops order so
  shift-click range selection is correct when sortOrder === 'oldest'
- onUpdateSelection prunes phantom IDs via a visibleIds filter so the
  select-all bar can't stay checked after an envelope disappears
- onBusSelectAllMatching emits section-selected after mass-select
  completes so Priority inbox siblings are cleared (the selection watcher
  skips this emit while loadingAllMatching is true)
- loadMore infinite-loop finding: already fixed (returns false on error,
  loop already breaks on false) — no change needed
- Markdown doc: label ASCII-diagram fences as text, update stale
  @update:checked examples to @update:model-value

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: RobinAngele <git@robin4consulting.com>
When loadMore() returns false (a page fetch error), the loop now sets a
loadFailed flag and aborts without populating this.selection from the
partial results. This prevents a silent partial selection being shown
as if it were complete.

Also enforce MAX_SELECT_MESSAGES as a hard cap on the final slice so
the selection never exceeds the limit regardless of the loop exit path.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: RobinAngele <git@robin4consulting.com>
@RobinAngele RobinAngele force-pushed the feat/filter-mass-select branch from 9f79425 to af4769f Compare May 10, 2026 09:53
@RobinAngele
Copy link
Copy Markdown
Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 10, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@RobinAngele RobinAngele force-pushed the feat/filter-mass-select branch from 7fad4f3 to 9d1ac5f Compare May 10, 2026 09:57
The mailbox() and sortOrder() watchers already reset expanded, but
searchQuery() was missing it. After a filter change the component could
start in a stale "expanded" state, causing the cached page to be shown
instead of triggering a fresh fetch.

Signed-off-by: RobinAngele <git@robin4consulting.com>
@RobinAngele RobinAngele force-pushed the feat/filter-mass-select branch from 9d1ac5f to ccc59be Compare May 10, 2026 09:58
Toast "Previous section selection was cleared" was wrongly appearing
when the 500-message cap was hit during a concurrent Priority inbox
mass-select (search modal triggers all sections simultaneously). Two
root causes:

1. The explicit section-selected emit in onBusSelectAllMatching was
   firing for all paths, including the concurrent bus-triggered path
   where sibling sections are also mass-selecting. Fixed by gating the
   emit behind emitSectionSelected=true, which is only passed by the
   single-section banner path (selectAllMatchingAction). The search
   modal path defaults to false so concurrent sections don't clear each
   other on completion.

2. The selection watcher was firing after loadingAllMatching=false
   (Vue flushes async watchers after the finally block), emitting
   section-selected even for programmatic mass-selects. Fixed by adding
   !selectAllMatching guard — only user-initiated transitions (where
   selectAllMatching is false) trigger mutual exclusion.

Also fix the 500-cap warning color: var(--color-warning) is an
amber background color, not a readable text color. Switch to dark text
on warning background for proper contrast in all themes.

Signed-off-by: RobinAngele <git@robin4consulting.com>
Two issues in the unified inbox mass-select flow:

1. 'No messages in this folder' flashing: when loadEnvelopes fetches
   with a new query the store briefly returns [] for that key, causing
   hasMessages=false and EmptyMailbox to render. Guard both empty-state
   branches with !loadingAllMatching so they only show when we're not
   mid mass-select. Also deduplicate v-if/else-if keys to satisfy the
   Vue unique-key lint rule.

2. Wrong or missing selection count: flatEnvelopeList uses the
   groupEnvelopes prop (passed from MailboxThread's computed). Prop
   updates go through Vue's render cycle and may lag one tick behind
   the store after loadMore() returns. Add await $nextTick() before
   reading flatEnvelopeList so the prop has fully propagated before
   we build the selection array.

Signed-off-by: RobinAngele <git@robin4consulting.com>
'Previous section selection was cleared' should only notify the user
when a manual selection is cleared — not when a programmatic mass-select
set is superseded. Capture selectAllMatching before clearing it in
onBusSectionSelected and gate the toast on wasUserInitiated.

Also reset selectAllMatching in onUpdateSelection / onSelectRange so
that manually modifying a selection after mass-select re-enables the
section-selected bus signalling for proper mutual exclusion.

Signed-off-by: RobinAngele <git@robin4consulting.com>
AI-assisted: Cline (Claude)
The differenceWith().forEach(env => flags.selected = false) block in
EnvelopeList's sortedEnvelops watcher was dead code: vanished envelopes
are already removed from the rendered list, so mutating their flags
has no effect. The preceding cleanedSelection emit already handles
the selection cleanup correctly. Remove the block and the now-unused
ramda differenceWith import.

Also add language identifiers to three unlabelled fenced code blocks
in the documentation (text, text, http) to pass markdown lint.

Signed-off-by: RobinAngele <git@robin4consulting.com>
AI-assisted: Cline (Claude)
@RobinAngele
Copy link
Copy Markdown
Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 11, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
src/components/Mailbox.vue (2)

962-968: 💤 Low value

Consider whether resetting expanded on unselect is the desired UX.

unselectAll() resets expanded = false (line 967), which collapses the list back to the initial page size. Users who scrolled to load more messages and then unselect might expect the list to remain expanded.

If this is intentional (clean reset), the current behavior is fine. If users should retain their scroll position and loaded messages, consider removing the expanded = false line.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/Mailbox.vue` around lines 962 - 968, The unselectAll() method
currently forces the message list to collapse by setting this.expanded = false;
to preserve the user's loaded/expanded view when they clear selection, remove
the this.expanded = false assignment from unselectAll() (leaving selection,
selectAllMatching, loadingAllMatching, and selectionLimitReached resets intact);
if collapse-on-unselect is indeed desired, instead add a comment above
unselectAll() explaining that collapsing is intentional for a full reset so
future readers know this side-effect is expected.

426-426: 💤 Low value

Minor typo in log message.

"initalization" should be "initialization".

-			logger.debug(`syncing folder ${this.mailbox.databaseId} (${this.searchQuery}) during cache initalization`)
+			logger.debug(`syncing folder ${this.mailbox.databaseId} (${this.searchQuery}) during cache initialization`)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/Mailbox.vue` at line 426, Fix the typo in the debug log
message in Mailbox.vue: update the logger.debug call that references
this.mailbox.databaseId and this.searchQuery to use "initialization" instead of
"initalization" so the log reads "syncing folder ${this.mailbox.databaseId}
(${this.searchQuery}) during cache initialization".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/components/Mailbox.vue`:
- Around line 962-968: The unselectAll() method currently forces the message
list to collapse by setting this.expanded = false; to preserve the user's
loaded/expanded view when they clear selection, remove the this.expanded = false
assignment from unselectAll() (leaving selection, selectAllMatching,
loadingAllMatching, and selectionLimitReached resets intact); if
collapse-on-unselect is indeed desired, instead add a comment above
unselectAll() explaining that collapsing is intentional for a full reset so
future readers know this side-effect is expected.
- Line 426: Fix the typo in the debug log message in Mailbox.vue: update the
logger.debug call that references this.mailbox.databaseId and this.searchQuery
to use "initialization" instead of "initalization" so the log reads "syncing
folder ${this.mailbox.databaseId} (${this.searchQuery}) during cache
initialization".

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5c7ed04a-9626-463e-acd5-63dab2e7983a

📥 Commits

Reviewing files that changed from the base of the PR and between 9f79425 and de89197.

📒 Files selected for processing (3)
  • Nextcloud_Mail_Select_All_and_Mass_Selection.md
  • src/components/EnvelopeList.vue
  • src/components/Mailbox.vue
✅ Files skipped from review due to trivial changes (1)
  • Nextcloud_Mail_Select_All_and_Mass_Selection.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/components/EnvelopeList.vue

v-if="isAtLeastOneSelectedFavorite"
variant="tertiary"
:title="n('mail', 'Unfavorite {number}', 'Unfavorite {number}', selection.length, { number: selection.length })"
@click.prevent="unfavoriteAll">
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Remnant of #12899.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Agreed — this hunk belongs to #12899. It appears here because this PR is stacked on top of that branch. Once #12899 merges and this branch is rebased onto main, the line will no longer be in this diff.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry to ask this, but am I talking to Robin or the agent? The PR is merged.

@@ -0,0 +1,509 @@
# Nextcloud Mail — Select All & Mass Selection Feature
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Drop this file.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done — dropped in commit b4c53ba.

… label

The Favorites section always has searchQuery = 'is:starred' (or
appended to a user filter). Without this exception, hasFilter returns
true for 'is:starred' alone, causing the select-all label to read
"Select N matching messages" instead of "Select all N messages" when
no user filter is active.

Export favoritesQuery from priorityInbox.js (alongside
priorityImportantQuery / priorityOtherQuery) so MailboxThread and
Mailbox share a single source of truth for the constant.

Signed-off-by: RobinAngele <git@robin4consulting.com>
AI-assisted: Cline (Claude)
…manual sections

totalEnvelopeCount always equalled flatEnvelopeList.length for scroll-paginated
mailboxes (both are backed by the same getEnvelopes() store call), making the
banner v-if condition permanently false. Replace the count comparison with the
direct !endReached flag, which already captures "more server pages exist".

selectAllHint said "Scroll down to include more messages" for paginate=manual
sections (Favorites, all Priority inbox sections). Those sections use a "Load
more" button. Suppress the hint for isPriorityInbox sections — they don't need
the guidance and the scroll wording was misleading there.

AI-assisted: Cline (Claude)
Signed-off-by: RobinAngele <git@robin4consulting.com>
Using isPriorityInbox as a proxy was coincidentally correct but semantically
wrong. The actual invariant is paginate === 'manual': those sections already
show a "Load more" button, so "Scroll down to include more messages" is
misleading. Check paginate directly so any future manual-paginate section
gets the right behaviour without needing isPriorityInbox.

AI-assisted: Cline (Claude)
Signed-off-by: RobinAngele <git@robin4consulting.com>
Requested by @ChristophWurst in PR review.

AI-assisted: Cline (Claude)
Signed-off-by: RobinAngele <git@robin4consulting.com>
Fix "initalization" → "initialization" in cache-init debug log.

Add comment to unselectAll() explaining why expanded is reset: collapsing
back to the initial page on full deselection avoids leaving a large loaded
list with nothing selected, which looks broken on re-entry.

AI-assisted: Cline (Claude)
Signed-off-by: RobinAngele <git@robin4consulting.com>
@RobinAngele RobinAngele force-pushed the feat/filter-mass-select branch 2 times, most recently from 34d862b to 0408e03 Compare May 11, 2026 18:19
- Spinner row text changed from "Selecting messages…" (same as the
  checkbox label) to "Loading all matching messages…" so the two
  concurrent messages are distinct during mass-select.
- Limit warning: "Too many messages — only the first {count} could be
  selected." → "Selection limited to {count} messages — use a filter to
  narrow results, or process in several batches." (frames it as a system
  cap and gives actionable workarounds).
- Banner button now shows "Select all messages in this folder" when no
  filter is active, matching the banner text; previously always said
  "Select all matching messages" regardless of context.
- Priority inbox section-cleared toast: "Previous section selection was
  cleared" → "Selection cleared — only one Priority inbox section can
  be selected at a time" (explains the reason).
- Hint text: "Scroll down to include" → "Scroll down to load", and
  "use filter" → "use a filter" for article consistency.
- Add showError() to both silent failure paths in onBusSelectAllMatching:
  page-load failure mid-loop and the outer catch block previously logged
  only to the console, leaving the user with a vanished spinner and no
  explanation. Both now show an error toast.

Signed-off-by: RobinAngele <git@robin4consulting.com>
- Spinner row text changed from "Selecting messages…" (same as the
  checkbox label) to "Loading all matching messages…" so the two
  concurrent messages are distinct during mass-select.
- Limit warning: "Too many messages — only the first {count} could be
  selected." → "Selection limited to {count} messages — use a filter to
  narrow results, or process in several batches." Frames it as a system
  cap and gives actionable workarounds.
- Banner button now shows "Select all messages in this folder" when no
  filter is active, matching the banner text; previously always said
  "Select all matching messages" regardless of context.
- Priority inbox section-cleared toast: "Previous section selection was
  cleared" → "Selection cleared — only one Priority inbox section can
  be selected at a time." Explains the reason.
- Hint text: "Scroll down to include" → "Scroll down to load", and
  "use filter" → "use a filter" for article consistency.
- Add showError() to both silent failure paths in onBusSelectAllMatching:
  page-load failure mid-loop and the outer catch block previously only
  logged to the console, leaving the user with a vanished spinner and no
  explanation. Also reset selectAllMatching in the catch block to restore
  mutual exclusion signalling and re-enable the select-all banner.

Signed-off-by: RobinAngele <git@robin4consulting.com>
@RobinAngele RobinAngele force-pushed the feat/filter-mass-select branch from e024338 to 088a749 Compare May 12, 2026 05:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Multi select is broken across date groups Implement a "Select All" menu item Select all messages in mailbox

2 participants