Skip to content

Conversation

@Simon-Laux
Copy link
Member

@Simon-Laux Simon-Laux commented Oct 8, 2025

TODO

  • we should figure out how to make async dialogs using suspense (with error handling/boundary) to ensure that the dialog does not resize/flicker when opened.
  • use useRpcFetch where it makes sense -> especially for getting the first contact.
  • changlog entry

depends on chatmail/core#7282

@WofWca
Copy link
Member

WofWca commented Oct 8, 2025

The TODO doesn't sound like a blocker? The flicker only happens once when you open it, right?

@Simon-Laux
Copy link
Member Author

Simon-Laux commented Oct 8, 2025

The TODO doesn't sound like a blocker? The flicker only happens once when you open it, right?

technically no, but I personally think that it is a big enough UX regression to investigate solutions. (I'm super annoyed by the flickering)

@Simon-Laux
Copy link
Member Author

seems like the flickering also breaks the e2e test

@WofWca
Copy link
Member

WofWca commented Oct 8, 2025

seems like the flickering also breaks the e2e test

I'll take a look.

@WofWca
Copy link
Member

WofWca commented Oct 8, 2025

This is not about flickering. It's because all members are shown as "Past members":

6f66d15ef0b6300b9876c82c9238d7d2433e917e

@Simon-Laux Simon-Laux force-pushed the simon/adapt-to-fullchat-contacts-removal branch from 4f318ed to 6cf3a12 Compare October 8, 2025 11:48
Copy link
Member

@WofWca WofWca left a comment

Choose a reason for hiding this comment

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

I checked that the moved code hasn't changed.


const firstChatContact = useRpcFetch(BackendRemote.rpc.getContact, [
accountId,
chat.contactIds[0],
Copy link
Member

Choose a reason for hiding this comment

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

This could be undefined, which would result in an error. Behavior-wise it's OK, but it will spam the console:

.catch(err => {
if (!outdated) {
setResultAndFetchId({
result: { ok: false, err },
fetchId,
})
log.errorWithoutStackTrace(
'error while executing fetch',
err,
fetchId
)
}
})

An "ideal" solution would be to pass null instead of [accountId, contactId]touseRpcFetch`.

@WofWca
Copy link
Member

WofWca commented Oct 8, 2025

About the flicker: I'm not sure if "suspense" is really what we need. We know the number of contact beforehand, so why not just display placeholders in place of them? Or at least one div of height depending on the number of contacts..

@Simon-Laux Simon-Laux changed the title adapt to FullChat.contacts removal refactor: adapt to FullChat.contacts removal Oct 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance Related to (improving) performance wait-for-core Waiting for an related core issue to be resolved

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants