Skip to content

Commit

Permalink
Fix contact selection issues
Browse files Browse the repository at this point in the history
This is another spin on solving #7256.

This reverts 5be8367 and 26dfa0f as they introduced some
issues with initialization, namely `initOnce` could run
multiple times.

In this commit we try to acknowledge that existing URL pattern is not
sufficient to distinguish between the cases as we might have selection
without focus.

Instead, we introduce an explicit argument to focus the item details.

We considered synchronizing the column focus state using this argument
in all cases but this would entail a larger refactoring.
  • Loading branch information
charlag authored and wec43 committed Aug 9, 2024
1 parent 8e8f2c0 commit 6226e69
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 34 deletions.
14 changes: 5 additions & 9 deletions src/mail-app/contacts/view/ContactView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -812,19 +812,15 @@ export class ContactView extends BaseTopLevelView implements TopLevelView<Contac
}

onNewUrl(args: Record<string, any>) {
const isSingleColumnLayout = styles.isSingleColumnLayout()
if (this.inContactListView()) {
this.contactListViewModel.showListAndEntry(args.listId, args.Id).then(m.redraw)
} else if (args.listId == null && args.contactId == null) {
// Redirect to the contacts view with the selected contact stored in the models if no arguments are given
this.contactViewModel.updateUrl(!isSingleColumnLayout)
} else {
this.contactViewModel.init(isSingleColumnLayout, args.listId, args.contactId)
this.contactViewModel.init(args.listId).then(() => this.contactViewModel.selectContact(args.contactId))
}
// focus the details column if asked explicitly, e.g. to show a specific contact
if (args.focusItem) {
this.viewSlider.focus(this.detailsColumn)
}

// Show the details of the contact on mobile instead of just all contacts
const isWithContact = !(args.Id == null && args.contactId == null)
if (isWithContact) this.viewSlider.focus(this.detailsColumn)
}

private deleteSelectedContacts(): Promise<void> {
Expand Down
55 changes: 31 additions & 24 deletions src/mail-app/contacts/view/ContactViewModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { ListModel } from "../../../common/misc/ListModel.js"
import { Contact, ContactTypeRef } from "../../../common/api/entities/tutanota/TypeRefs.js"
import { compareContacts } from "./ContactGuiUtils.js"
import { ListState } from "../../../common/gui/base/List.js"
import { assertNotNull, memoized } from "@tutao/tutanota-utils"
import { assertNotNull, lazyMemoized } from "@tutao/tutanota-utils"
import { GENERATED_MAX_ID, getElementId } from "../../../common/api/common/utils/EntityUtils.js"
import Stream from "mithril/stream"
import { Router } from "../../../common/gui/ScopedRouter.js"
Expand All @@ -15,6 +15,8 @@ import { ListAutoSelectBehavior } from "../../../common/misc/DeviceConfig.js"
/** ViewModel for the overall contact view. */
export class ContactViewModel {
contactListId!: Id
/** id of the contact we are trying to load based on the url */
private targetContactId: Id | null = null
sortByFirstName: boolean = true
private listModelStateStream: Stream<unknown> | null = null

Expand All @@ -41,44 +43,43 @@ export class ContactViewModel {
autoSelectBehavior: () => ListAutoSelectBehavior.NONE,
})

async init(isSingleColumnLayout: boolean, contactListId?: Id, contactId?: Id) {
this.contactListId = contactListId ? contactListId : await this.getContactListId()
async init(contactListId?: Id) {
// update url if the view was just opened
if (contactListId == null) this.updateUrl()
if (this.contactListId) return

this.listModel.loadInitial().then(async () => {
// we are loading all contacts at once anyway so we are not worried about starting parallel loads for target
typeof contactId === "string" && (await this.loadAndSelect(contactId))
})
this.contactListId = assertNotNull(await this.contactModel.getContactListId(), "not available for external users")

this.initOnce()
await this.listModel.loadInitial()
}

this.initOnce(isSingleColumnLayout)
async selectContact(contactId: Id) {
// We are loading all contacts at once anyway so we are not worried about starting parallel loads for target
await this.loadAndSelect(contactId)
}

private readonly initOnce = memoized((isSingleColumnLayout: boolean) => {
private readonly initOnce = lazyMemoized(() => {
this.eventController.addEntityListener(this.entityListener)
this.listModelStateStream = this.listModel.stateStream.map(() => {
this.updateUi()
this.updateUrl(!isSingleColumnLayout) // Avoid keyboard up and down opening the details column in single column layout
this.updateUrl()
})
})

// Redirects the browser to the contact route with the currently selected contact list and contact as parameters
async updateUrl(willLoadContact: boolean) {
private updateUrl() {
const contactId =
!this.listModel.state.inMultiselect && this.listModel.getSelectedAsArray().length === 1
this.targetContactId ??
(!this.listModel.state.inMultiselect && this.listModel.getSelectedAsArray().length === 1
? getElementId(this.listModel.getSelectedAsArray()[0])
: null
const listId = this.contactListId ? this.contactListId : await this.getContactListId()
if (contactId && willLoadContact) {
this.router.routeTo(`/contact/:listId/:contactId`, { listId, contactId })
: null)
if (contactId) {
this.router.routeTo(`/contact/:listId/:contactId`, { listId: this.contactListId, contactId: contactId })
} else {
this.router.routeTo(`/contact/:listId`, { listId })
this.router.routeTo(`/contact/:listId`, { listId: this.contactListId })
}
}

// Gets the ContactListId from the contact model
private async getContactListId(): Promise<Id> {
return assertNotNull(await this.contactModel.getContactListId(), "not available for external users")
}

private readonly entityListener: EntityEventsListener = async (updates) => {
for (const update of updates) {
if (isUpdateForTypeRef(ContactTypeRef, update) && update.instanceListId === this.contactListId) {
Expand All @@ -89,7 +90,13 @@ export class ContactViewModel {

async loadAndSelect(contactId: Id) {
const listId = this.contactListId
await this.listModel.loadAndSelect(contactId, () => this.contactListId !== listId)
this.targetContactId = contactId

await this.listModel.loadAndSelect(contactId, () => this.contactListId !== listId && this.targetContactId === contactId)
// if we reached the goal and the target wasn't swapped in between
if (this.targetContactId === contactId) {
this.targetContactId = null
}
}

setSortByFirstName(sorting: boolean) {
Expand Down
2 changes: 1 addition & 1 deletion src/mail-app/mail/view/MailViewer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,7 @@ export class MailViewer implements Component<MailViewerAttrs> {
label: "showContact_action",
click: () => {
const [listId, contactId] = assertNotNull(contact)._id
m.route.set("/contact/:listId/:contactId", { listId, contactId })
m.route.set("/contact/:listId/:contactId", { listId, contactId, focusItem: true })
},
})
} else {
Expand Down

0 comments on commit 6226e69

Please sign in to comment.