Add key prop to NclistItem to re-render conversation with the appropr…

[DorraJaouad]

☑️ Resolves

Sometimes ( not sure what the trigger is yet), the active conversation class gets passed to all conversation items in the virtual scroller one by one successively.

Adding a key prop to NcListItem will force proper re-render.

🚧 Tasks

• code review

[Antreesy]

Was lucky to capture the screencast (at 00:06):

Screencast.from.26.09.2023.11.30.26.webm

Happens mostly around the active conversation component, active state is handled via <RouterLink/>

Also wanted to note, that we have duplicated hidden conversation items in the list. To check:

document.querySelectorAll('.conversation-item').forEach(item => console.log(item.getAttribute('data-nav-id')))

Not happening for the Demo component:
https://vue-virtual-scroller-demo.netlify.app/recycle

document.querySelectorAll('.td.index').forEach(item => console.log(item.textContent))

Not sure, that a force-rerendering with :key will help with performance. Furthermore, maybe it's hidden conversations that triggering it, cc @ShGKme for some thoughts

[ShGKme]

Was lucky to capture the screencast:

Was it uploaded correctly? I see only this static image in the video...

Anyway, is there a way to reproduce the bug? So we can debug it to find what causes the problem. Or at least a screenshot with a bug =D

Not sure, that a force-rerendering with :key will help with performance. Furthermore, maybe it's hidden conversations that triggering it, cc @ShGKme for some thoughts

As NcListItem doesn't have any local state (except NcAvatar which already has a key, unfortunately), it won't break anything. But it makes scrolling a less performance. Component instances are not reused anymore but are fully re-created (including DOM elements) during the scrolling.

[DorraJaouad]

Pause on 0:05

glitching142016.mp4

[ShGKme]

Yep, I can see the problem now. Though, still cannot reproduce it...

[ShGKme]

Furthermore, maybe it's hidden conversations that triggering it

Having duplicated IDs is also weird. but it should not trigger an active state change. Vue Router adds an active state to all active links even if there are many.

[DorraJaouad]

Tested when switching to filters as the glitching is also visible there.

With key	Without key

[ShGKme]

Still cannot reproduce to debug, let's go with the key for now

[nickvergessen]

Also fixes #10698 ?

If so we should backport this to stable27?

[ShGKme]

Also fixes #10698 ?

If so we should backport this to stable27?

Yes.

The problem with titles is caused by the direct DOM mutation here:

spreed/src/components/LeftSidebar/ConversationsList/Conversation.vue

Lines 339 to 341 in 6de7100

	if (titleSpan && titleSpan.offsetWidth < titleSpan.scrollWidth) {
	titleSpan.setAttribute('title', value)
	}

Vue is not aware of this change and doesn't update the title because it is not bind to the application state.

key fix this indirectly by destroying and creating a DOM node from scratch.

[nickvergessen]

/backport to stable27

[nickvergessen]

/backport to stable27