LotusGuild/cinny
3
0
Fork 0
Code Issues 27 Pull Requests 5 Actions Packages Projects Releases Wiki Activity

Reactions row flickers between "wrapped" and "one row + N" every frame when the chips almost fit, and the collapsed style wipes the row's top margin #305

New Issue
Closed
opened 2026-10-04 23:25:53 -04:00 by jared · 0 comments
jared commented 2026-10-04 23:25:53 -04:00
Owner
Copy Link
Copy Source

The one-row reactions collapse with a "+N" chip (#138, bf05751e) has two layout bugs in src/app/features/room/message/Reactions.tsx. Both come from the inline style applied while collapsed (line 114):

{ maxHeight: rowHeight + 4, overflow: 'hidden', padding: 2, margin: -2 }

1. Endless collapse/expand loop when the chips are within 4 px of fitting

measure() uses container.getBoundingClientRect().width (the border box) as the row width. While collapsed, margin: -2 widens the border box by 4 px and padding: 2 keeps the content where it was. So the available width reads 4 px larger while collapsed than while not. If the chips' one-row width falls in that 4 px gap, the cycle never ends:

  1. Not collapsed, row width 671 px: the chips don't fit, setLimit(9), and the row collapses.
  2. Collapsed, row width 675 px: the height changes, the ResizeObserver fires, measure() finds that everything fits and calls setLimit(undefined).
  3. The row is no longer collapsed, the height changes again, the observer fires, and the cycle returns to step 1. This repeats every frame.

Measured in Chromium on a message with 12 reactions ("react to me a lot", local test room), sampling every animation frame for 2 s:

viewport container states seen layouts / 2 s script time / 2 s
1440 px (all chips fit) w=673.6 h=26 0 4 ms
1349 px w=671 h=60, no margin ↔ w=675 h=30, margin -2px 33 216 ms
  • Where it happens: sweeping the viewport 1200→1500 px one pixel at a time, the loop occurs at exactly 1348–1351 px, which is the 4 px gap.
  • Firefox: its font metrics put the same message into the gap at the default 1440×900. The dev console logs about 10 Removing a style property during rerender (margin) when a conflicting property is set (marginTop) warnings per second, and a MutationObserver counts about 40 style flips per second on that one div while the app is idle. In Chromium the same page at 1440 px is quiet.

Why it matters:

  • Above roughly 1440 px the timeline column is capped at about 674 px. On wide screens, whether a message loops therefore depends on its reaction set and fonts, not on the window size. A message that loops does so for everyone with the same browser and fonts, for as long as it is on screen.
  • The row visibly flickers between two rows and one row + "+N". The message height jumps 30 px on every frame, so the content around it shifts (scroll anchoring), and the "+N" chip appears and disappears under the cursor.
  • The loop costs about 11 % of a core's main-thread time per looping message (battery drain on laptops and phones). Several such messages on screen multiply the cost.

2. The collapsed style overrides the timeline's marginTop, then deletes it

The timeline passes style={{ marginTop: config.space.S200 }} to <Reactions> (RoomTimeline.tsx:1286, 1417, 1537, 1600, 1653, ThreadTimeline.tsx:839). The collapsed style spreads margin: -2 after it. Measured gap between the message body and the reactions row:

state inline style gap above the row
normal (fits) margin-top: var(--…) 8 px
collapsed (+N) margin: -2px; max-height: 30px; … −2 px (chips sit right under the text, see screenshot 2)
un-collapsed again (e.g. window widened) (empty) 0 px, until the message remounts

When React removes the margin shorthand it also clears margin-top. It doesn't re-apply marginTop, because that prop didn't change, which is what the React warning above is about. So after any collapse→expand cycle the row stays glued to the message. This happens when the window or panel width changes, when a thread or members drawer opens or closes, or when a reaction is added and the row fits again.

Fix direction

  • Loop: measure the width the chips actually get, e.g. container.clientWidth minus horizontal padding, or subtract the 4 px while collapsed. Alternatively apply the same padding: 2; margin: 0 -2px in both states so the width doesn't depend on the state. A 1–2 px hysteresis in fitReactionRow is a cheap extra guard.
  • Margin: don't use the margin shorthand. Use marginLeft/Right: -2 plus marginTop: calc(<S200> - 2px), or keep the outline room with padding and an inner wrapper so the caller's marginTop is left alone.
  • Tests: a unit test where the chips' width is between rowWidth and rowWidth + 4, and a Playwright check that the row's style doesn't change for 1 s while idle at a width inside that gap (for this fixture, 1349 px in Chromium).

Screenshot

reactions-gap-1-normal.png

reactions-gap-2-collapsed.png

reactions-gap-3-after-uncollapse.png

Found in the October 2026 client audit (Playwright against a local Synapse + LiveKit stack, plus code reading). Code refs are at cinny 30e2dbcd, cinny-desktop 8f67ec7, element-call 3b0050e8.

The one-row reactions collapse with a "+N" chip (#138, bf05751e) has two layout bugs in `src/app/features/room/message/Reactions.tsx`. Both come from the inline style applied while collapsed (line 114): ```ts { maxHeight: rowHeight + 4, overflow: 'hidden', padding: 2, margin: -2 } ``` ### 1. Endless collapse/expand loop when the chips are within 4 px of fitting `measure()` uses `container.getBoundingClientRect().width` (the border box) as the row width. While collapsed, `margin: -2` widens the border box by 4 px and `padding: 2` keeps the content where it was. So the available width reads 4 px larger while collapsed than while not. If the chips' one-row width falls in that 4 px gap, the cycle never ends: 1. Not collapsed, row width 671 px: the chips don't fit, `setLimit(9)`, and the row collapses. 2. Collapsed, row width 675 px: the height changes, the `ResizeObserver` fires, `measure()` finds that everything fits and calls `setLimit(undefined)`. 3. The row is no longer collapsed, the height changes again, the observer fires, and the cycle returns to step 1. This repeats every frame. **Measured** in Chromium on a message with 12 reactions ("react to me a lot", local test room), sampling every animation frame for 2 s: | viewport | container states seen | layouts / 2 s | script time / 2 s | |---|---|---|---| | 1440 px (all chips fit) | `w=673.6 h=26` | 0 | 4 ms | | 1349 px | `w=671 h=60, no margin` ↔ `w=675 h=30, margin -2px` | 33 | 216 ms | - **Where it happens:** sweeping the viewport 1200→1500 px one pixel at a time, the loop occurs at exactly 1348–1351 px, which is the 4 px gap. - **Firefox:** its font metrics put the same message into the gap at the default 1440×900. The dev console logs about 10 `Removing a style property during rerender (margin) when a conflicting property is set (marginTop)` warnings per second, and a MutationObserver counts about 40 style flips per second on that one div while the app is idle. In Chromium the same page at 1440 px is quiet. **Why it matters:** - Above roughly 1440 px the timeline column is capped at about 674 px. On wide screens, whether a message loops therefore depends on its reaction set and fonts, not on the window size. A message that loops does so for everyone with the same browser and fonts, for as long as it is on screen. - The row visibly flickers between two rows and one row + "+N". The message height jumps 30 px on every frame, so the content around it shifts (scroll anchoring), and the "+N" chip appears and disappears under the cursor. - The loop costs about 11 % of a core's main-thread time per looping message (battery drain on laptops and phones). Several such messages on screen multiply the cost. ### 2. The collapsed style overrides the timeline's `marginTop`, then deletes it The timeline passes `style={{ marginTop: config.space.S200 }}` to `<Reactions>` (`RoomTimeline.tsx:1286, 1417, 1537, 1600, 1653`, `ThreadTimeline.tsx:839`). The collapsed style spreads `margin: -2` after it. Measured gap between the message body and the reactions row: | state | inline style | gap above the row | |---|---|---| | normal (fits) | `margin-top: var(--…)` | **8 px** | | collapsed (+N) | `margin: -2px; max-height: 30px; …` | **−2 px** (chips sit right under the text, see screenshot 2) | | un-collapsed again (e.g. window widened) | *(empty)* | **0 px**, until the message remounts | When React removes the `margin` shorthand it also clears `margin-top`. It doesn't re-apply `marginTop`, because that prop didn't change, which is what the React warning above is about. So after any collapse→expand cycle the row stays glued to the message. This happens when the window or panel width changes, when a thread or members drawer opens or closes, or when a reaction is added and the row fits again. ### Fix direction - **Loop:** measure the width the chips actually get, e.g. `container.clientWidth` minus horizontal padding, or subtract the 4 px while collapsed. Alternatively apply the same `padding: 2; margin: 0 -2px` in both states so the width doesn't depend on the state. A 1–2 px hysteresis in `fitReactionRow` is a cheap extra guard. - **Margin:** don't use the `margin` shorthand. Use `marginLeft/Right: -2` plus `marginTop: calc(<S200> - 2px)`, or keep the outline room with `padding` and an inner wrapper so the caller's `marginTop` is left alone. - **Tests:** a unit test where the chips' width is between `rowWidth` and `rowWidth + 4`, and a Playwright check that the row's style doesn't change for 1 s while idle at a width inside that gap (for this fixture, 1349 px in Chromium). **Screenshot** ![reactions-gap-1-normal.png](https://code.lotusguild.org/attachments/b68e1538-d681-4b95-a8b9-e63c4263ebc2) ![reactions-gap-2-collapsed.png](https://code.lotusguild.org/attachments/5cac9f33-9170-4fd2-bc71-1ffdc638c300) ![reactions-gap-3-after-uncollapse.png](https://code.lotusguild.org/attachments/68169e34-1b9b-4460-a31f-289cb570ed5b) --- _Found in the October 2026 client audit (Playwright against a local Synapse + LiveKit stack, plus code reading). Code refs are at cinny `30e2dbcd`, cinny-desktop `8f67ec7`, element-call `3b0050e8`._
reactions-gap-1-normal.png
28 KiB
reactions-gap-2-collapsed.png
20 KiB
reactions-gap-3-after-uncollapse.png
27 KiB
jared added the bugpriority: mediumperformancearea: messaging labels 2026-10-04 23:25:53 -04:00
jared closed this issue 2026-10-17 20:07:28 -04:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
a11y
Accessibility: keyboard, screen reader, contrast, motion
 area: appearance
Client area: appearance
 area: auth-session
Client area: auth-session
 area: build-ci
Client area: build-ci
 area: calls
Client area: calls
 area: desktop
Client area: desktop
 area: media
Client area: media
 area: messaging
Client area: messaging
 area: mobile
Client area: mobile
 area: moderation
Client area: moderation
 area: navigation
Client area: navigation
 area: notifications
Client area: notifications
 area: settings
Client area: settings
 area: threads
Client area: threads
 bug
Something is not working
 dependencies
Third-party package versions and advisories
 docs
README / LOTUS_* docs wrong or missing
 duplicate
This issue or pull request already exists
 enhancement
New feature
 help wanted
Need some help
 invalid
Something is wrong
 needs-human-review
performance
Re-render storms, leaks, heavy work on hot paths
 planning
priority: critical
Data loss, security hole, or crash on a main path
 priority: high
Broken feature or serious usability problem
 priority: low
Minor issue or polish
 priority: medium
Wrong behaviour in an edge case or notable degradation
 qa
Manual QA: shipped, needs a human in a real environment
 question
More information is needed
 research
security
XSS, unsafe URLs, data leaks, auth/session
 tech-debt
Code health, dead code, fragile patterns
 ux
Usability or visual inconsistency
 wontfix
This won't be fixed
No labels area: messaging bug performance priority: medium
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
cole (Cole Keller) jared (Jared Vititoe) nathan (Nathan Vititoe)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: LotusGuild/cinny#305