Skip to content

Commit 2d03fd2

Browse files
wrdhubwrdhub
committed
Apply only the labels that have actually been added or removed
Create LabelsPopupViewModel, add tests Close #10166
1 parent 62e1e15 commit 2d03fd2

File tree

6 files changed

+450
-127
lines changed

6 files changed

+450
-127
lines changed

‎src/mail-app/mail/view/LabelsPopup.ts‎

Lines changed: 43 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -8,32 +8,30 @@ import { component_size, size } from "../../../common/gui/size.js"
88
import { AllIcons, Icon, IconSize } from "../../../common/gui/base/Icon.js"
99
import { Icons } from "../../../common/gui/base/icons/Icons.js"
1010
import { theme } from "../../../common/gui/theme.js"
11-
import { Keys, MAX_LABELS_PER_MAIL, TabIndex } from "../../../common/api/common/TutanotaConstants.js"
11+
import { Keys, TabIndex } from "../../../common/api/common/TutanotaConstants.js"
1212
import { getElementId } from "../../../common/api/common/utils/EntityUtils.js"
1313
import { getLabelColor } from "../../../common/gui/base/Label.js"
1414
import { LabelState } from "../model/MailModel.js"
1515
import { AriaRole } from "../../../common/gui/AriaUtils.js"
1616
import { lang } from "../../../common/misc/LanguageViewModel.js"
1717
import { noOp } from "@tutao/tutanota-utils"
18+
import { LabelsPopupViewModel } from "./LabelsPopupViewModel"
1819

1920
/**
2021
* Popup that displays assigned labels and allows changing them
2122
*/
2223
export class LabelsPopup implements ModalComponent {
2324
private dom: HTMLElement | null = null
24-
private isMaxLabelsReached: boolean
2525

2626
constructor(
2727
private readonly sourceElement: HTMLElement,
2828
private readonly origin: PosRect,
2929
private readonly width: number,
30-
private readonly labelsForMails: ReadonlyMap<Id, ReadonlyArray<MailSet>>,
31-
private readonly labels: { label: MailSet; state: LabelState }[],
30+
private readonly viewModel: LabelsPopupViewModel,
3231
private readonly onLabelsApplied: (addedLabels: MailSet[], removedLabels: MailSet[]) => unknown,
3332
) {
3433
this.view = this.view.bind(this)
3534
this.oncreate = this.oncreate.bind(this)
36-
this.isMaxLabelsReached = this.checkIsMaxLabelsReached()
3735
}
3836

3937
async hideAnimation(): Promise<void> {}
@@ -69,49 +67,47 @@ export class LabelsPopup implements ModalComponent {
6967
[
7068
m(
7169
".pb-8.scroll",
72-
this.labels
73-
.sort((labelA, labelB) => labelA.label.name.localeCompare(labelB.label.name))
74-
.map((labelState) => {
75-
const { label, state } = labelState
76-
const color = theme.on_surface
77-
const canToggleLabel = state === LabelState.Applied || state === LabelState.AppliedToSome || !this.isMaxLabelsReached
78-
const opacity = !canToggleLabel ? 0.5 : undefined
79-
80-
return m(
81-
"label-item.flex.items-center.plr-12.state-bg.cursor-pointer",
82-
83-
{
84-
"data-labelid": getElementId(label),
85-
role: AriaRole.MenuItemCheckbox,
86-
tabindex: TabIndex.Default,
87-
"aria-checked": ariaCheckedForState(state),
88-
"aria-disabled": !canToggleLabel,
89-
onclick: canToggleLabel ? () => this.toggleLabel(labelState) : noOp,
90-
},
91-
[
92-
m(Icon, {
93-
icon: this.iconForState(state),
94-
size: IconSize.PX24,
70+
this.viewModel.getLabelState().map((labelState) => {
71+
const { label, state } = labelState
72+
const color = theme.on_surface
73+
const canToggleLabel = state === LabelState.Applied || state === LabelState.AppliedToSome || !this.viewModel.isLabelLimitReached()
74+
const opacity = !canToggleLabel ? 0.5 : undefined
75+
76+
return m(
77+
"label-item.flex.items-center.plr-12.state-bg.cursor-pointer",
78+
79+
{
80+
"data-labelid": getElementId(label),
81+
role: AriaRole.MenuItemCheckbox,
82+
tabindex: TabIndex.Default,
83+
"aria-checked": ariaCheckedForState(state),
84+
"aria-disabled": !canToggleLabel,
85+
onclick: canToggleLabel ? () => this.viewModel.toggleLabel(label) : noOp,
86+
},
87+
[
88+
m(Icon, {
89+
icon: this.iconForState(state),
90+
size: IconSize.PX24,
91+
style: {
92+
fill: getLabelColor(label.color),
93+
opacity,
94+
},
95+
}),
96+
m(
97+
".button-height.flex.items-center.ml-12.overflow-hidden",
98+
{
9599
style: {
96-
fill: getLabelColor(label.color),
100+
color,
97101
opacity,
98102
},
99-
}),
100-
m(
101-
".button-height.flex.items-center.ml-12.overflow-hidden",
102-
{
103-
style: {
104-
color,
105-
opacity,
106-
},
107-
},
108-
m(".text-ellipsis", label.name),
109-
),
110-
],
111-
)
112-
}),
103+
},
104+
m(".text-ellipsis", label.name),
105+
),
106+
],
107+
)
108+
}),
113109
),
114-
this.isMaxLabelsReached && m(".small.center.pb-8", lang.get("maximumLabelsPerMailReached_msg")),
110+
this.viewModel.isLabelLimitReached() ? m(".small.center.pb-8", lang.get("maximumLabelsPerMailReached_msg")) : null,
115111
m(BaseButton, {
116112
label: "apply_action",
117113
text: lang.get("apply_action"),
@@ -143,48 +139,8 @@ export class LabelsPopup implements ModalComponent {
143139
}
144140
}
145141

146-
private checkIsMaxLabelsReached(): boolean {
147-
const { addedLabels, removedLabels } = this.getSortedLabels()
148-
if (addedLabels.length >= MAX_LABELS_PER_MAIL) {
149-
return true
150-
}
151-
152-
for (const [, labels] of this.labelsForMails) {
153-
const labelsOnMail = new Set<Id>(labels.map((label) => getElementId(label)))
154-
155-
for (const label of removedLabels) {
156-
labelsOnMail.delete(getElementId(label))
157-
}
158-
if (labelsOnMail.size >= MAX_LABELS_PER_MAIL) {
159-
return true
160-
}
161-
162-
for (const label of addedLabels) {
163-
labelsOnMail.add(getElementId(label))
164-
if (labelsOnMail.size >= MAX_LABELS_PER_MAIL) {
165-
return true
166-
}
167-
}
168-
}
169-
170-
return false
171-
}
172-
173-
private getSortedLabels(): Record<"addedLabels" | "removedLabels", MailSet[]> {
174-
const removedLabels: MailSet[] = []
175-
const addedLabels: MailSet[] = []
176-
for (const { label, state } of this.labels) {
177-
if (state === LabelState.Applied) {
178-
addedLabels.push(label)
179-
} else if (state === LabelState.NotApplied) {
180-
removedLabels.push(label)
181-
}
182-
}
183-
return { addedLabels, removedLabels }
184-
}
185-
186142
private applyLabels() {
187-
const { addedLabels, removedLabels } = this.getSortedLabels()
143+
const { addedLabels, removedLabels } = this.viewModel.getLabelStateChange()
188144
this.onLabelsApplied(addedLabels, removedLabels)
189145
modal.remove(this)
190146
}
@@ -193,7 +149,7 @@ export class LabelsPopup implements ModalComponent {
193149
this.dom = vnode.dom as HTMLElement
194150

195151
// restrict label height to showing maximum 6 labels to avoid overflow
196-
const displayedLabels = Math.min(this.labels.length, 6)
152+
const displayedLabels = Math.min(this.viewModel.getLabelState().length, 6)
197153
const height = (displayedLabels + 1) * component_size.button_height + size.spacing_8 * 2
198154
showDropdown(this.origin, this.dom, height, this.width).then(() => {
199155
const firstLabel = vnode.dom.getElementsByTagName("label-item").item(0)
@@ -243,10 +199,7 @@ export class LabelsPopup implements ModalComponent {
243199
exec: () => {
244200
const labelId = document.activeElement?.getAttribute("data-labelid")
245201
if (labelId) {
246-
const labelItem = this.labels.find((item) => getElementId(item.label) === labelId)
247-
if (labelItem) {
248-
this.toggleLabel(labelItem)
249-
}
202+
this.viewModel.toggleLabelById(labelId)
250203
} else {
251204
return true
252205
}
@@ -258,22 +211,6 @@ export class LabelsPopup implements ModalComponent {
258211
show() {
259212
modal.displayUnique(this, false)
260213
}
261-
262-
private toggleLabel(labelState: { label: MailSet; state: LabelState }) {
263-
switch (labelState.state) {
264-
case LabelState.AppliedToSome:
265-
labelState.state = this.isMaxLabelsReached ? LabelState.NotApplied : LabelState.Applied
266-
break
267-
case LabelState.NotApplied:
268-
labelState.state = LabelState.Applied
269-
break
270-
case LabelState.Applied:
271-
labelState.state = LabelState.NotApplied
272-
break
273-
}
274-
275-
this.isMaxLabelsReached = this.checkIsMaxLabelsReached()
276-
}
277214
}
278215

279216
function ariaCheckedForState(state: LabelState): string {

‎src/mail-app/mail/view/LabelsPopupViewModel.ts‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
import { MailSet } from "../../../common/api/entities/tutanota/TypeRefs"
2+
import { LabelState } from "../model/MailModel"
3+
import { MAX_LABELS_PER_MAIL } from "../../../common/api/common/TutanotaConstants"
4+
import { getElementId } from "../../../common/api/common/utils/EntityUtils"
5+
6+
export class LabelsPopupViewModel {
7+
private labelLimitReached: boolean = false
8+
private labelStateTracker: Array<{ label: MailSet; state: LabelState; startingState: LabelState }> = []
9+
10+
constructor(
11+
private readonly labelsForMails: ReadonlyMap<Id, ReadonlyArray<MailSet>>,
12+
initialLabelState: { label: MailSet; state: LabelState }[],
13+
) {
14+
for (const label of initialLabelState) {
15+
this.labelStateTracker.push({ label: label.label, state: label.state, startingState: label.state })
16+
}
17+
// We sort right now
18+
this.labelStateTracker.sort((labelA, labelB) => labelA.label.name.localeCompare(labelB.label.name))
19+
this.updateLabelLimitReached()
20+
}
21+
22+
getLabelState(): Array<{ label: MailSet; state: LabelState; startingState: LabelState }> {
23+
return this.labelStateTracker
24+
}
25+
26+
isLabelLimitReached(): boolean {
27+
return this.labelLimitReached
28+
}
29+
30+
toggleLabel(label: MailSet): void {
31+
this.toggleLabelById(getElementId(label))
32+
}
33+
34+
toggleLabelById(labelId: Id): void {
35+
const labelState = this.labelStateTracker.find((item) => getElementId(item.label) === labelId)
36+
if (!labelState) {
37+
return
38+
}
39+
switch (labelState.state) {
40+
case LabelState.AppliedToSome:
41+
labelState.state = this.labelLimitReached ? LabelState.NotApplied : LabelState.Applied
42+
break
43+
case LabelState.NotApplied:
44+
if (!this.labelLimitReached) {
45+
labelState.state = LabelState.Applied
46+
}
47+
break
48+
case LabelState.Applied:
49+
labelState.state = LabelState.NotApplied
50+
break
51+
}
52+
53+
this.updateLabelLimitReached()
54+
}
55+
56+
getLabelStateChange(): Record<"addedLabels" | "removedLabels", MailSet[]> {
57+
const addedLabels: MailSet[] = []
58+
const removedLabels: MailSet[] = []
59+
60+
for (const label of this.labelStateTracker) {
61+
if (label.state !== label.startingState) {
62+
if (label.state === LabelState.Applied) {
63+
addedLabels.push(label.label)
64+
} else if (label.state === LabelState.NotApplied) {
65+
removedLabels.push(label.label)
66+
}
67+
// Do not have to check for LabelState.AppliedToSome, it cannot be explicitly set
68+
}
69+
}
70+
71+
return { addedLabels, removedLabels }
72+
}
73+
74+
private updateLabelLimitReached(): void {
75+
const { addedLabels, removedLabels } = this.getLabelStateChange()
76+
if (addedLabels.length >= MAX_LABELS_PER_MAIL) {
77+
this.labelLimitReached = true
78+
return
79+
}
80+
81+
for (const [, labels] of this.labelsForMails) {
82+
// creating a set removes duplicates in the case of a label getting added to all that was previously only on some
83+
const labelsOnMail = new Set([...addedLabels, ...labels])
84+
85+
if (labelsOnMail.size - removedLabels.length >= MAX_LABELS_PER_MAIL) {
86+
this.labelLimitReached = true
87+
return
88+
}
89+
}
90+
91+
this.labelLimitReached = false
92+
}
93+
}

‎src/mail-app/mail/view/MailGuiUtils.ts‎

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5,28 +5,13 @@ import { Dialog } from "../../../common/gui/base/Dialog"
55
import { AllIcons } from "../../../common/gui/base/Icon"
66
import { Icons } from "../../../common/gui/base/icons/Icons"
77
import { isApp, isDesktop } from "../../../common/api/common/Env"
8-
import {
9-
$Promisable,
10-
assertNotNull,
11-
clamp,
12-
endsWith,
13-
filterInt,
14-
first,
15-
isEmpty,
16-
isNotEmpty,
17-
lazyMemoized,
18-
neverNull,
19-
noOp,
20-
promiseMap,
21-
} from "@tutao/tutanota-utils"
8+
import { $Promisable, assertNotNull, clamp, filterInt, first, isEmpty, isNotEmpty, lazyMemoized, neverNull, noOp, promiseMap } from "@tutao/tutanota-utils"
229
import {
2310
EncryptionAuthStatus,
2411
getMailFolderType,
2512
MailReportType,
2613
MailSetKind,
27-
MailState,
2814
SimpleMoveMailTarget,
29-
SYSTEM_GROUP_MAIL_ADDRESS,
3015
SystemFolderType,
3116
} from "../../../common/api/common/TutanotaConstants"
3217
import { getReportConfirmation } from "./MailReportDialog"
@@ -66,6 +51,7 @@ import { DownloadListener, TransferProgressDispatcher } from "../../../common/ap
6651
import stream from "mithril/stream"
6752
import { showProgressDialog } from "../../../common/gui/dialogs/ProgressDialog"
6853
import { CancelledError } from "../../../common/api/common/error/CancelledError"
54+
import { LabelsPopupViewModel } from "./LabelsPopupViewModel"
6955

7056
const UNDO_SNACKBAR_SHOW_TIME = 10 * 1000 // ms
7157

@@ -816,8 +802,7 @@ export function showLabelsPopup(
816802
dom ?? (document.activeElement as HTMLElement),
817803
opts?.origin ?? dom?.getBoundingClientRect() ?? getDetachedDropdownBounds(),
818804
opts?.width ?? (styles.isDesktopLayout() ? 300 : 200),
819-
mailModel.getLabelsForMails(selectedMails),
820-
mailModel.getLabelStatesForMails(selectedMails),
805+
new LabelsPopupViewModel(mailModel.getLabelsForMails(selectedMails), labels),
821806
async (addedLabels, removedLabels) => mailModel.applyLabels(await getActionableMails(selectedMails), addedLabels, removedLabels),
822807
)
823808
setTimeout(() => popup.show(), 16)

‎src/mail-app/mail/view/MailViewerHeader.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ import { px, size } from "../../../common/gui/size.js"
3434
import { highlightTextInQueryAsChildren } from "../../../common/gui/TextHighlightViewUtils"
3535
import { EventBanner, EventBannerAttrs } from "./EventBanner"
3636
import { getGroupColors } from "../../../common/misc/GroupColors"
37-
3837
import { getTimeFormatForUser } from "../../../common/api/common/utils/UserUtils"
38+
import { LabelsPopupViewModel } from "./LabelsPopupViewModel"
3939

4040
export type MailAddressDropdownCreator = (args: {
4141
mailAddress: MailAddressAndName
@@ -872,8 +872,10 @@ export class MailViewerHeader implements Component<MailViewerHeaderAttrs> {
872872
dom,
873873
dom.getBoundingClientRect(),
874874
styles.isDesktopLayout() ? 300 : 200,
875-
viewModel.mailModel.getLabelsForMails([viewModel.mail]),
876-
viewModel.mailModel.getLabelStatesForMails([viewModel.mail]),
875+
new LabelsPopupViewModel(
876+
viewModel.mailModel.getLabelsForMails([viewModel.mail]),
877+
viewModel.mailModel.getLabelStatesForMails([viewModel.mail]),
878+
),
877879
(addedLabels, removedLabels) => viewModel.mailModel.applyLabels([viewModel.mail._id], addedLabels, removedLabels),
878880
)
879881
// waiting for the dropdown to be closed

‎test/tests/Suite.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ import "./mail/SpamClassificationHandlerTest.js"
190190
import "./misc/quickactions/QuickActionsModelTest.js"
191191
import "./calendar/CalendarTimeGridTest"
192192
import "./calendar/AllDaySectionTest"
193+
import "./mail/view/LabelsPopupViewModelTest.js"
193194

194195
import * as td from "testdouble"
195196
import { random } from "@tutao/tutanota-crypto"

0 commit comments

Comments
 (0)