Repository navigation
feat(ui-message-list): add the message list engine - #43728
gabriellsh wants to merge 39 commits into
Conversation
🦋 Changeset detectedLatest commit: a4a2aa4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 18 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## message-list/06-variants #43728 +/- ##
============================================================
- Coverage 71.20% 70.48% -0.73%
============================================================
Files 5565 3938 -1627
Lines 247439 216833 -30606
Branches 46375 35714 -10661
============================================================
- Hits 176196 152826 -23370
+ Misses 66869 62434 -4435
+ Partials 4374 1573 -2801
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…answers Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…y scrollbars message-list R78, R79. Co-Authored-By: Claude <noreply@anthropic.com>
…nversation fixtures message-list R79. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
message-list R2, R5 to R11, R17, R30, R36 to R40, R59 to R63. Co-Authored-By: Claude <noreply@anthropic.com>
…sage list message-list R12 to R47, R54 to R56, R69. D4 is pinned by a skipped test. Co-Authored-By: Claude <noreply@anthropic.com>
message-list R66 to R68. Co-Authored-By: Claude <noreply@anthropic.com>
…ponent specs message-list R1 to R9, R12, R19 to R24, R41 to R53, R57 to R59, R62, R64 to R66, R70, R77, R78. D1 and D3 are pinned by skipped tests. Co-Authored-By: Claude <noreply@anthropic.com>
…list message-list R13 to R40, R44, R47, R51 to R57, R60, R61, R64, R66, R69, R74, R75. Co-Authored-By: Claude <noreply@anthropic.com>
message-list R75, R76. Co-Authored-By: Claude <noreply@anthropic.com>
… its saved position message-list R61. The test that fails first: "opens at the saved position when its messages arrive after it mounted (R61)", and the RestorePositionOfAListThatMountsEmpty play. Co-Authored-By: Claude <noreply@anthropic.com>
…ot from what the list holds message-list R56. The play that fails first: ReportsBeingAtTheBottomAfterAJump. Co-Authored-By: Claude <noreply@anthropic.com>
…changes size message-list R55. The reducer row and the play that fail first: ReportsAfterAResize. Co-Authored-By: Claude <noreply@anthropic.com>
… the bottom message-list R17, R31, R80. The room's list follows arrivals within 60px and the thread's within 20px, as they do today. Co-Authored-By: Claude <noreply@anthropic.com>
…after its page was committed message-list R34, D4: the pinned test is un-skipped and its entry removed. Co-Authored-By: Claude <noreply@anthropic.com>
…s changed message-list R71 to R73. The spec that fails first: "renders only the new row when a message arrives (R72)". Co-Authored-By: Claude <noreply@anthropic.com>
…n arrow asked for message-list R66. The spec that fails first: "keeps trying for a few frames while the virtualizer renders the row (R66)". The KeyboardIntoUnrenderedRows play now starts from a kept-mounted row far from the viewport. Co-Authored-By: Claude <noreply@anthropic.com>
…ne, not the nearest rendered one message-list R66. A message kept mounted far from the viewport sits among the rendered ones, so an arrow from it reached a row far away. The play that fails first: KeyboardFromAKeptMountedRow. Co-Authored-By: Claude <noreply@anthropic.com>
…sked for message-list R20. Without onJumpToLatest the list kept waiting for the latest page and went to the bottom by itself when a later commit ended the newer pages. The rejected-promise case stays D3. The specs that fail first: the reducer row and "does not move by itself when the latest page cannot be asked for, now or when the messages next stop having a newer page (R20)". Co-Authored-By: Claude <noreply@anthropic.com>
The feature docs, the frontend guide, the note on the package's copy of the keyboard hooks, the two index entries and the ADR of virtua and the scroll reducer. Co-Authored-By: Claude <noreply@anthropic.com>
…nted Co-Authored-By: Claude <noreply@anthropic.com>
…, the empty report, the selection and the leaving of the keyboard Found by changing the code under the specs and watching them still pass: message-list R11, R49, R58, R67, R70, R72. Co-Authored-By: Claude <noreply@anthropic.com>
…oves Co-Authored-By: Claude <noreply@anthropic.com>
…n before reading it message-list R56. ReportsNotBeingAtTheBottomOfAnOlderWindow read the report before it re-rendered and passed without the newer-loader condition; it now holds the loads and waits for the top message to change. Changing the code under it makes it fail. Co-Authored-By: Claude <noreply@anthropic.com>
…ot only arrive past it message-list R80. A resize that leaves a gap smaller than the threshold must still bring the list to the true bottom; re-anchoring that stops within the threshold now fails BottomThreshold. Co-Authored-By: Claude <noreply@anthropic.com>
…irst messages are listed message-list R61, R62. Branch 08 drops the position when the first messages show something unread; a position given only then was ignored. The spec that fails first: "applies the position given when the first messages are listed, not the one given at mount (R61, R62)". Co-Authored-By: Claude <noreply@anthropic.com>
…ping Each play that slept now waits for the state it reads: the loads settled, the position reported, the bubble hidden. The input story is split in three so no window of the viewer's scrolling is left open for the next. The floating date is allowed 600 ms to hide, since it would otherwise show for a second: that second is the behaviour under test. Changing the code under the plays makes them fail. Co-Authored-By: Claude <noreply@anthropic.com>
… plan done Co-Authored-By: Claude <noreply@anthropic.com>
… first review Co-Authored-By: Claude <noreply@anthropic.com>
…rive late or it gets a size A list whose first page arrived after it had stopped placing itself committed that page while idle: it asked for an older page from the top of the list in the same commit that sent it to the bottom, and a list shown after it had no size did the same. It now places itself on those messages, and on its new size, before it checks for a page. A list that already showed rows, its header or a loader, stays where it is for its first messages, unless they open on a restored message. The reducer rows and the FirstPageArrivesLate play failed before the change. message-list R29. Co-Authored-By: Claude <noreply@anthropic.com>
…tself The list kept the position it stood at while it was still on its way to the bottom or to a restored message, so a room left in those first frames was stored near the top and reopened there. The list keeps a position only once it has placed itself. The spec that unmounts a list still placing itself failed before the change. message-list R59. Co-Authored-By: Claude <noreply@anthropic.com>
…it loads pages The room's list loaded no page before the viewer's first wheel, pointer press or scroll key, and again after a jump rebuilt its window, so a room that opens loads nothing it was not asked for. The list takes deferPagingUntilInput for that: off by default, and while on, no page loads before the viewer's input (R25's, or a pointer press in the list), the first input checks for a page at once, and a replaced window waits for input again. The reducer rows and the PagingWaitsForInput plays failed with the gate taken out. message-list R82, R34. Co-Authored-By: Claude <noreply@anthropic.com>
…ast one The room's list kept 21px above its first row and 10px below its last. Padding around the rows would leave the list short of its bottom, which is where the last row ends, and scrolling back to it on every resize; the space is part of the first and last rows instead, so the list rests at its bottom with the space in view, follows arrivals and resizes to it, and keeps the rows in place when a page lands at the start. The first page's spinner sits below the space above. startSpacing and endSpacing are 0 by default. The SpaceAtTheEnds play failed with the space as padding around the rows. message-list R83, R14. Co-Authored-By: Claude <noreply@anthropic.com>
…laced itself A list that opens at the bottom, or on the message it was left at, was seen moving there: the rows showed from the first frame while the heights the virtualizer had guessed were measured and the bottom was corrected over several frames. The rows stay mounted and measured but transparent while the list places itself, and show once it is placed, at the viewer's first input, when the focus enters the list, or half a second after it started; the list is busy meanwhile. Rows the viewer already saw never go out of sight again, so an empty conversation showing its header stays in sight when its first message arrives. The OpensWhereItLands play, which records every frame of an opening, failed with the rows in sight; the FirstMessageStaysInSight play, which records every frame of a first message, failed with the header hidden; the specs of the half-second deadline, the spinner, the busy state and the focus each failed with its part taken out. message-list R84, R1. Co-Authored-By: Claude <noreply@anthropic.com>
…der as the room's list did The room's floating date showed on every scroll, its own or not, stood in for the day divider under it, which it hid, was pushed up by the next day's divider, and faded a second after the last scroll unless that divider lay above the top of the list and below the top of the window. The list's date only showed while the viewer scrolled and hid under a divider instead. It now follows those rules again. Each scroll places it from the dividers drawn near the top into a small store only the date reads, so no row draws again. The placement rows and the FloatingDate and FloatingDateNeverDoublesADay plays failed with each rule taken out. message-list R51, R52, R27. Co-Authored-By: Claude <noreply@anthropic.com>
…er draws over the list The room drew its floating date below the unread banner and the upload progress, and measured the day dividers from there. The list takes overlayTopInset, the height a consumer draws over its top, 0 by default: the date rests that far below the top, and stands in for, is pushed by and stays for the dividers measured from that line. The FloatingDateBelowAnInset play failed with the inset left out of the placement and with it left out of where the date rests; the placement row failed with it left out. message-list R85. Co-Authored-By: Claude <noreply@anthropic.com>
910f0b3 to
a4a2aa4
Compare
Proposed changes (including videos or screenshots)
Adds the list engine to
@rocket.chat/ui-message-list: the row model, the scroll reducer and scroller, the keyboard model, theMessageListwith its date bubble, new-messages and jump-to-latest overlays, stories with play tests, and the docs (docs/features/message-list/,docs/frontend/message-list.md, two entries inCLAUDE.mdand ADR 0001 in the package). Nothing inapps/meteoruses the list yet; this part only adds it with its tests.Enabling commits are at the bottom:
useFormatDateand a viewport mode for the overlay scrollbars in@rocket.chat/ui-client, and deterministic fixtures in@rocket.chat/mock-providers. The list is copied from the reference implementation and then fixed where it would not hold for the room and thread: a position given before the first messages are listed,isAtBottomread from the position, a visibility report on resize, abottomThresholdoption, a repeated check for a due page, row redraws only when what a row shows changed, an arrow key that reaches the adjacent row and keeps trying for a few frames, and no waiting for a latest page that cannot be asked for. Each of these fixes is its own commit with a test that failed first.What the list does to match
develop's room and thread:develop's spinner (R3);aria-busymeanwhile; rows show at once on the viewer's first input or when the focus enters the list. A list that already showed rows, its header or a loader is never hidden again, and does not place itself again for its first messages (R29, R84);develop's rules: it shows on every scroll, hides the day divider it stands in for, is pushed up by the next day's divider, and fades out 1s after the last scroll, over 0.6s (R51, R52).New options, used by parts 08 and 09 of the stack:
deferPagingUntilInput, no page before the viewer's first input (R82);startSpacingandendSpacing, space above the first row and below the last one, counted as part of those rows (R83);overlayTopInset, the height a consumer draws over the top of the list, below which the floating date rests and measures, placed again at once when the inset changes (R85). The list also places itself again when its first messages arrive late or its viewport gets a size, and reports no position before it has placed itself (R29, R59).Spacing in the list uses the spacing scale tokens. The package spec is
docs/specs/message-list.md(R1 to R85; part 08 of the stack adds R81).This is part 07 of the stack (00 to 09). Review it against its base branch
message-list/06-variants, not againstdevelop.Stack
Visible changes
No change visible to users: the list is not used yet. No changeset for
@rocket.chat/ui-message-list..changeset/ui-client-viewport-scrollbars.md,'@rocket.chat/ui-client': minor: addsuseFormatDateanduseViewportScrollbars..changeset/mock-providers-fixtures.md,'@rocket.chat/mock-providers': minor: addscreateFakeUser,createFakeRoom,createFakeSubscription,createFakeMessageWithAttachmentandcreateFakeConversation.Known differences from develop
None. The package's own copy of the item-list keyboard hook follows the app's current hook; the app's copy is untouched.
Issue(s)
Steps to test or reproduce
Walk the
Components/MessageListstories (yarn workspace @rocket.chat/ui-message-list storybook) against the behaviors indocs/features/message-list/README.md. Each story is one behavior.LoadingStates,FirstPageLoadingandFirstPageLoadingBelowTheSpace: the loaders are 32px high withdevelop's spinner, and the first page's spinner shows at the top.OlderPageOfTheSameDayandInfiniteOlderHistory: an older page does not move what is on screen, and the next page loads only when the top is near again.PagingWaitsForInputandPagingWaitsForInputOnAShortPage: no page loads until the first wheel, scroll key or pointer press in the list.SpaceAtTheEnds: the space above the first row and below the last one stays as messages arrive and the viewport resizes.OpensWhereItLands: switching rooms shows no frame of rows anywhere but where the list lands. Press Tab into the list within half a second of opening a room: the rows show, and so does the focus.FirstMessageStaysInSight: the first message of an empty conversation arrives under its header and nothing blinks.FloatingDate,FloatingDateNeverDoublesADay,FloatingDateBelowAnInsetandFloatingDateFollowsTheInset: the date hides the divider it stands in for, is pushed up by the next divider, fades, rests below the inset, and moves at once when the inset changes.Further comments
Decisions to check
bottomThresholdoption on the list with a default of 1; parts 08 (room) and 09 (thread) of the stack pass 60 and 20. Reason: the base wins, and 1 px would stop following new messages for a viewer a few pixels off the bottom. If the reducer could not take it without breaking its invariants, the fallback was (a) with a changeset. Please check this.onJumpToLatestleaves the list waiting for it and it later jumps to the bottom by itself) are recorded with skipped tests inmessage-list.md. D3 is fixed in part 09 of the stack.develop's rule: the date stays without fading while the divider it stands in for is above the top of the list and below the top of the window. So when the list sits low in the window, a divider just out of view, inside the window's top margin, keeps the date on screen (R52).@rocket.chat/ui-message-list. This PR adds none for the private package; parts 08 and 09 of the stack do. The repo versions private packages (other private packages carry changesets), so 08 and 09 follow the repo, and 03 to 07 may need changesets too. Undecided.34e9c872c98above the test commits, anddf00a650826toa4a2aa4f0bbabove the commit that marks the spec implemented; each of those updates the spec and plan with its change. In the plays, a paused load and the floating date's fade run in real time on purpose.Not run
@rocket.chat/mock-providershas no typecheck script; its build runstsc.🤖 Generated with Claude Code