elk-zone / elk

A nimble Mastodon web client
https://elk.zone
MIT License
5.51k stars 564 forks source link

fix(ui): Avoid duplicate IDs for grouped notifications #3014

Closed palant closed 3 weeks ago

palant commented 1 month ago

Steps to reproduce are in https://github.com/elk-zone/elk/issues/2216#issuecomment-2424229983. This fixes #2216 and #2889.

The issue here is function groupItems issuing identical IDs for different objects. In the first update it creates a GroupedLikeNotifications instance with ID grouped-0. Then we get a second update, and it again creates a GroupedLikeNotifications instance with ID grouped-0. Since both have the same key, Vue attempts to reuse the existing node for the new object. It replaces the existing vnode in StatusBody by one that belongs to a different status, and it fails miserably updating the display. So we get a correct internal structure but a completely messed up display. The solution is using a global ID counter, so that this function never re-issues the same ID.

netlify[bot] commented 1 month ago

Deploy Preview for elk-docs canceled.

Name Link
Latest commit 1a2085c7174bc2be90ef5637a590e25d6055ebf8
Latest deploy log https://app.netlify.com/sites/elk-docs/deploys/6718048c0a22ce0008698bb4
netlify[bot] commented 1 month ago

Deploy Preview for elk-zone ready!

Name Link
Latest commit 1a2085c7174bc2be90ef5637a590e25d6055ebf8
Latest deploy log https://app.netlify.com/sites/elk-zone/deploys/6718048c35111f00081c11d6
Deploy Preview https://deploy-preview-3014--elk-zone.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

shuuji3 commented 3 weeks ago

Thank you for the detailed analysis! I can also confirm the fix with the reproduction steps. 💯