Conversation
|
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: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v2 #1368 +/- ##
=====================================
Coverage ? 38.15%
=====================================
Files ? 396
Lines ? 30265
Branches ? 0
=====================================
Hits ? 11548
Misses ? 18717
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The bar could only be reached by touch: a mouse cannot drag it, a vertical wheel does not move a horizontal list, and Material draws no scrollbar for one. A floating button sits at either end of the bar, appearing while there is something further that way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A chevron is all a screen reader had to go on. Adds `participantsBarPrevious` and `participantsBarNext` to the localizations, in English and Dutch, and hands them to the buttons as tooltips. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The grid drew its own 48px square chevron. Both layouts now share `ParticipantsNavigationButton`, and the grid follows the frames for where it sits: over the grid below the small breakpoint, either side of it above. `participantsBarPrevious`/`Next` lose the `Bar` — the grid pages rather than scrolls, and both layouts use them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Also hide scaled-away navigation buttons from screen readers, and inset each small-grid chevron by its own side's padding. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
renefloor
force-pushed
the
feat/participants-bar-scroll-buttons
branch
from
September 24, 2026 13:25
50ce2e4 to
737b421
Compare
renefloor
marked this pull request as ready for review
September 24, 2026 13:25
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
On desktop the participants bar in
CallParticipantsSpotlightViewwas unreachable past its first screenful:MaterialScrollBehavior.dragDevicesexcludesPointerDeviceKind.mouse, so it cannot be dragged with a mouse.ScrollabletakesscrollDelta.dxfor a horizontal axis.MaterialScrollBehavior.buildScrollbarreturns the child unchanged forAxis.horizontal, so there is no scrollbar either.With 10 participants in a 1280-wide window on
speakerTop, the 222×125 tiles run to 2288px and about five of the ten sit off screen with nothing indicating they exist. #1361 fixing the tile size is what turned that from rare into routine above roughly six people.What
A floating button at either end of the participants bar, appearing while there is something further that way. A tap brings the tile cut off at that end to the other end, as far in as the list's padding, so no tile stays half hidden under a button. Works on all four
ParticipantsBarAlignmentvalues — left/right chevrons for a horizontal bar, up/down for a vertical one, swapped under RTL.The grid's page chevrons become the same button, since the grid frames draw the same component. Both layouts share
ParticipantsNavigationButton, and the grid's old 48×48 squarePageNavigationButtonis gone.Design
Both Figma frames (627-13954, 627-13942) are
Core / Floating Button Wrapper: a 40×40 circle oncore/background/core/elevation-1withshadow/web/light/elevation-2and a 16px icon, inset 12px (spacing.sm) from each end of the bar.That did not match the grid's old
PageNavigationButton(48×48, radius 8,barsBg, no shadow), so the grid was brought onto the same component rather than the bar growing a second one.Where the button sits is per-layout, and each number is measured off a frame:
spacing.sm) in from each endA narrow grid keeps its full width because it has none to spare; above
StreamScreenSize.small(768, matching the frames) the tiles are set in between the buttons instead. A hidden button holds its place either way, so the grid is the same size on every page.Each of those insets is to the button's visual, which sits inside a wider tap target, so the layouts take
participantsNavigationButtonTapInsetoff their own inset to place it.The frames wrap
Core / Web / Button, butstream_core_flutterhas no web/mobile split —Core / Mobile / Buttoncarries the same "min-width and min-height are enforced via hit-target variables" note, which in Flutter isStreamButton's defaulttapTargetSize: .padded. So the button isStreamButton.icon(style: .secondary, type: .ghost, isFloating: true)withiconSize: 16, keeping the 48px hit target around the 40px visual, and the 12px inset is measured to the visual.Notes for review
ValueNotifierfed by aScrollMetricsNotificationlistener (layout changes: the first layout, a participant joining, the view resizing) and aScrollNotificationlistener (scrolling).ParticipantsNavigationButtontakesvisibleand hides itself:AnimatedScale0/1, which also drops it out of the hit test since a zero transform is not invertible, plusExcludeSemantics, because a zero-scale button is otherwise still read out.CallParticipantsGridView.paddingis nowEdgeInsetsGeometryso it accepts directional insets.update_goldens):spotlight_smallchanged,spotlight_bar_buttonsis new (a horizontal and a vertical overflowing bar), andgrid_pagedis new (a narrow window and a wider one, since every existing grid golden fits on one page). Both runs rewrote only those files, so neither layout moves wherever the buttons are not shown.flutter test, verified by snapshottingchevronRight,chevronDown,moreHorizontaland Flutter's ownIcons.chevron_righttogether — all four come out as tofu.participantsPrevious/participantsNextlocalizations in English and Dutch — layout-neutral, since the grid pages rather than scrolls.StreamButton.icon'stooltipputs the string on the semantics node, the way Flutter's ownIconButtondoes.Testing
New cases in
call_participants_spotlight_view_test.dart: all four alignments, both ends, landing on a tile edge each way, hidden while the tiles fit, reacting to a participant joining or the window growing, clamping at the end, geometry horizontal, vertical and RTL, a tap passing through a hidden button, and that each label lands on the right button's semantics.New cases in
call_participants_grid_view_test.dart: the labels, which chevrons show on the first and last page, going back, a hidden chevron not being read out, RTL paging and uneven RTL padding, the grid keeping its size across pages, and the 16 and the 8/40/8 off the frames above.CI=true flutter test test/src/call_participants/passes all 274.🤖 Generated with Claude Code