-
Notifications
You must be signed in to change notification settings - Fork 13.9k
chore(video-conf): conference window, unmounted (2/4) #41956
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
1548ff0
6e75f46
1d73cfb
3949c5a
7c2db19
a12f52d
0c741b0
d6e770a
1cf0e56
bbdb59f
f1e48c0
eb215dd
262eb00
0ac0863
0f9150f
ca3e12e
48e42ef
1e75ce7
ac7d861
b5bcf83
351e43d
8ea4a60
f683597
40599f5
893c7f4
432d4e7
a289913
9707e7d
e0b2029
faba010
465f4bc
183c4b4
656a297
c3c690a
f1f1326
8e9f7aa
639fd6b
ce87094
9770402
b25249c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| --- | ||
| '@rocket.chat/fuselage-ui-kit': patch | ||
| '@rocket.chat/mock-providers': patch | ||
| '@rocket.chat/desktop-api': patch | ||
| '@rocket.chat/ui-client': patch | ||
| '@rocket.chat/ui-kit': patch | ||
| '@rocket.chat/i18n': patch | ||
| '@rocket.chat/meteor': patch | ||
| --- | ||
|
|
||
| Groundwork for the video conference window: no user-facing change. | ||
|
|
||
| Adds the window's views and hooks, an ongoing-calls list, and the small shared-package additions they need. None of it is mounted into the application — no item appears in the navigation bar, the room header, the room list or the message list, no existing screen renders anything new, and no new request, poll or subscription starts. The window is reachable only by visiting a `/conference/...` URL directly, a route that already existed and until now rendered a placeholder. The feature that mounts all of this arrives separately, behind a setting that is off by default. | ||
|
|
||
| Three shared behaviours changed along the way: `UserAction.addStream` reference-counts its subscribers instead of throwing when a room is already streaming, so the same room can be mounted twice; `AuthenticationCheck` and `UsernameCheck` accept a `loading` placeholder so a standalone route need not flash the app-shaped skeleton; and `GenericMenuItem` accepts a `textValue` so a menu item with rendered content is still announced and matched by typeahead. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| import { PaletteStyleTag } from '@rocket.chat/fuselage'; | ||
| import { mockAppRoot } from '@rocket.chat/mock-providers'; | ||
| import { render } from '@testing-library/react'; | ||
|
|
||
| import AppLayoutThemeWrapper from './AppLayoutThemeWrapper'; | ||
|
|
||
| const APP_TAG_ID = 'app-layout-palette'; | ||
| const REFERENCE_TAG_ID = 'reference-palette'; | ||
|
|
||
| const cssOf = (tagId: string) => document.getElementById(tagId)?.textContent; | ||
|
|
||
| /** The palette Fuselage itself emits for a theme, so the assertions below aren't a copy of its colour values. */ | ||
| const paletteFor = (theme: 'light' | 'dark' | 'high-contrast') => { | ||
| const { unmount } = render(<PaletteStyleTag theme={theme} tagId={REFERENCE_TAG_ID} />); | ||
| const css = cssOf(REFERENCE_TAG_ID); | ||
| unmount(); | ||
| return css; | ||
| }; | ||
|
|
||
| const renderWrapper = (themeAppearence: string, theme?: 'dark') => | ||
| render(<AppLayoutThemeWrapper theme={theme}>{null}</AppLayoutThemeWrapper>, { | ||
| wrapper: mockAppRoot().withUserPreference('themeAppearence', themeAppearence).build(), | ||
| }); | ||
|
|
||
| describe('AppLayoutThemeWrapper', () => { | ||
| it('pins the palette it is given, whatever the reader prefers', () => { | ||
| renderWrapper('light', 'dark'); | ||
|
|
||
| expect(cssOf(APP_TAG_ID)).toBe(paletteFor('dark')); | ||
| }); | ||
|
|
||
| // The conference window pins dark because a call surface is dark. High contrast is not a look but a | ||
| // legibility need, so it is the one preference the pin gives way to. | ||
| it('gives way to high contrast', () => { | ||
| renderWrapper('high-contrast', 'dark'); | ||
|
|
||
| expect(cssOf(APP_TAG_ID)).toBe(paletteFor('high-contrast')); | ||
| }); | ||
|
|
||
| // Guards every other route from the change above: left unpinned this still follows the system's dark mode — | ||
| // jsdom reports light — rather than the appearance preference, which is what the app's content palette reads. | ||
| it('follows the system for a layout that pins nothing', () => { | ||
| renderWrapper('dark'); | ||
|
|
||
| expect(cssOf(APP_TAG_ID)).toBe(paletteFor('light')); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,51 @@ | ||
| import { PaletteStyleTag } from '@rocket.chat/fuselage'; | ||
| import type { PaletteStyleTagProps } from '@rocket.chat/fuselage'; | ||
| import { useDarkMode } from '@rocket.chat/fuselage-hooks'; | ||
| import { useThemeMode } from '@rocket.chat/ui-client'; | ||
| import type { ReactNode } from 'react'; | ||
|
|
||
| export type AppLayoutThemeWrapperProps = { children: ReactNode }; | ||
| type PinnedTheme = PaletteStyleTagProps['theme']; | ||
|
|
||
| const AppLayoutThemeWrapper = ({ children }: AppLayoutThemeWrapperProps) => { | ||
| export type AppLayoutThemeWrapperProps = { | ||
| children: ReactNode; | ||
| /** | ||
| * Pins the palette instead of following the reader's appearance preference — for a layout whose look is part | ||
| * of what it is rather than a matter of taste. The conference window is the one that pins: a call surface is | ||
| * dark in every product that has one, and light controls over a black video tile read as a bug rather than | ||
| * as a light theme. | ||
| * | ||
| * High contrast is the exception a pin never overrides. Unlike light and dark it answers a legibility need, | ||
| * so it outranks whatever look a layout wants. | ||
| */ | ||
| theme?: PinnedTheme; | ||
| }; | ||
|
|
||
| /** | ||
| * Left unpinned this is exactly the light/dark the tag has always emitted, so every other route's palette is | ||
| * untouched: only a pinned layout consults the appearance preference at all, and only to let high contrast | ||
| * through. | ||
| */ | ||
| const resolveTheme = (pinned: PinnedTheme, mode: ReturnType<typeof useThemeMode>, dark: boolean): PinnedTheme => { | ||
| if (!pinned) { | ||
| return dark ? 'dark' : 'light'; | ||
| } | ||
|
|
||
| if (mode === 'high-contrast') { | ||
| return 'high-contrast'; | ||
| } | ||
|
|
||
| return pinned; | ||
| }; | ||
|
|
||
| const AppLayoutThemeWrapper = ({ children, theme: pinned }: AppLayoutThemeWrapperProps) => { | ||
| const dark = useDarkMode(); | ||
| const mode = useThemeMode(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: When a route does not pin a theme, this unconditional Prompt for AI agents |
||
|
|
||
| const theme = resolveTheme(pinned, mode, dark); | ||
|
|
||
| return ( | ||
| <> | ||
| <PaletteStyleTag theme={dark ? 'dark' : 'light'} tagId='app-layout-palette' /> | ||
| <PaletteStyleTag theme={theme} tagId='app-layout-palette' /> | ||
| {children} | ||
| </> | ||
| ); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| import { mockAppRoot } from '@rocket.chat/mock-providers'; | ||
| import { render, screen } from '@testing-library/react'; | ||
|
|
||
| import CallParticipants from './CallParticipants'; | ||
|
|
||
| const person = (username: string) => ({ _id: username, username }); | ||
|
|
||
| const renderParticipants = (props: Parameters<typeof CallParticipants>[0], avatars = true) => | ||
| render(<CallParticipants {...props} />, { | ||
| wrapper: mockAppRoot().withJohnDoe().withUserPreference('displayAvatars', avatars).build(), | ||
| }); | ||
|
|
||
| // Faces say *who* is in the call, which is usually what decides whether to join. The count alone never did. | ||
| it('shows a face for each of the people it was given', () => { | ||
| const { container } = renderParticipants({ people: [person('alice'), person('bob')], total: 2 }); | ||
|
|
||
| expect(container.querySelectorAll('img')).toHaveLength(2); | ||
| }); | ||
|
|
||
| // Said the way the call's own message block says it, so a call reads the same in the sidebar as in its room. | ||
| it('follows the faces with how many more there are', () => { | ||
| renderParticipants({ people: [person('alice'), person('bob'), person('carol')], total: 12 }); | ||
|
|
||
| expect(screen.getByText('plus__usersCount__joined')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('says just "joined" when every one of them is shown', () => { | ||
| renderParticipants({ people: [person('alice'), person('bob')], total: 2 }); | ||
|
|
||
| expect(screen.getByText('joined')).toBeInTheDocument(); | ||
| expect(screen.queryByText('plus__usersCount__joined')).not.toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('still says how many there are, for anyone who cannot see the faces', () => { | ||
| renderParticipants({ people: [person('alice')], total: 4 }); | ||
|
|
||
| expect(screen.getByTitle('__count__people_in_the_call')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| // A single face reads as a mistake rather than as a call, so an empty second place stands beside it — decorative, | ||
| // which is why it is hidden from anyone listening rather than looking. | ||
| it('gives a lone face an empty place beside it', () => { | ||
| const { container } = renderParticipants({ people: [person('alice')], total: 1 }); | ||
|
|
||
| expect(container.querySelectorAll('img')).toHaveLength(1); | ||
| expect(container.querySelectorAll('div[aria-hidden="true"]')).toHaveLength(1); | ||
| }); | ||
|
|
||
| it('leaves the empty place out once there are two', () => { | ||
| const { container } = renderParticipants({ people: [person('alice'), person('bob')], total: 2 }); | ||
|
|
||
| expect(container.querySelectorAll('div[aria-hidden="true"]')).toHaveLength(0); | ||
| }); | ||
|
|
||
| // Side by side rather than overlapped, so nothing has to be stacked and no face is half hidden behind another. | ||
| it('sets the faces beside each other', () => { | ||
| const people = [person('alice'), person('bob'), person('carol')]; | ||
| const { container } = renderParticipants({ people, total: 3 }); | ||
|
|
||
| expect(container.querySelectorAll('img')).toHaveLength(people.length); | ||
| expect(container.querySelector('[style*="z-index"]')).toBeNull(); | ||
| }); | ||
|
|
||
| // An older server, or a call whose members did not travel with it. | ||
| it('falls back to the number when there are no faces to show', () => { | ||
| const { container } = renderParticipants({ people: [], total: 3 }); | ||
|
|
||
| expect(screen.getByText('__usersCount__joined')).toBeInTheDocument(); | ||
| expect(container.querySelectorAll('img')).toHaveLength(0); | ||
| }); | ||
|
|
||
| // Avatars are a preference, and the message block honours it by saying the count in words instead. | ||
| it('says it in words when the reader has avatars turned off', () => { | ||
| const { container } = renderParticipants({ people: [person('alice'), person('bob')], total: 5 }, false); | ||
|
|
||
| expect(screen.getByText('__usersCount__joined')).toBeInTheDocument(); | ||
| expect(container.querySelectorAll('img')).toHaveLength(0); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| import { Box } from '@rocket.chat/fuselage'; | ||
| import type { Meta, StoryObj } from '@storybook/react'; | ||
|
|
||
| import CallParticipants from './CallParticipants'; | ||
| import { conferenceAppRoot, withCallProviders } from '../views/conference/storyFixtures'; | ||
|
|
||
| /** | ||
| * Who is already in a call: their faces, then how many more there are. | ||
| * | ||
| * Faces answer *who* is in there, which is usually what decides whether to walk in — so the interesting states | ||
| * are the ones where there are no faces to show. | ||
| */ | ||
| const meta = { | ||
| component: CallParticipants, | ||
| parameters: { layout: 'centered' }, | ||
| decorators: [ | ||
| (Story) => ( | ||
| <Box backgroundColor='surface-light' padding={12} borderRadius='x4'> | ||
| <Story /> | ||
| </Box> | ||
| ), | ||
| withCallProviders(conferenceAppRoot()), | ||
| ], | ||
| } satisfies Meta<typeof CallParticipants>; | ||
|
|
||
| export default meta; | ||
|
|
||
| type Story = StoryObj<typeof meta>; | ||
|
|
||
| const people = [ | ||
| { _id: 'ada', username: 'ada' }, | ||
| { _id: 'grace', username: 'grace' }, | ||
| { _id: 'alan', username: 'alan' }, | ||
| ]; | ||
|
|
||
| /** Everyone in the call has a face, so it ends with a plain "joined" rather than a count. */ | ||
| export const AllShown: Story = { | ||
| args: { people, total: 3 }, | ||
| }; | ||
|
|
||
| /** More in the call than faces shown: the rest become "+ N joined". */ | ||
| export const WithOverflow: Story = { | ||
| args: { people, total: 12 }, | ||
| }; | ||
|
|
||
| /** | ||
| * One person. A blank circle keeps its place, so a single face doesn't read as a lone dot against the text. | ||
| */ | ||
| export const SinglePerson: Story = { | ||
| args: { people: [people[0]], total: 1 }, | ||
| }; | ||
|
|
||
| /** The larger size, for a full screen rather than a sidebar row. */ | ||
| export const LargerAvatars: Story = { | ||
| args: { people, total: 7, size: 'x24' }, | ||
| }; | ||
|
|
||
| /** | ||
| * Avatars turned off in preferences. There is nobody to show, so it falls back to the count in words — the way | ||
| * the call's own message block says it. | ||
| */ | ||
| export const AvatarsDisabled: Story = { | ||
| args: { people, total: 7 }, | ||
| decorators: [withCallProviders(conferenceAppRoot().withUserPreference('displayAvatars', false))], | ||
| }; | ||
|
|
||
| /** | ||
| * A call whose members didn't travel with it — an older server. Nobody has a username, so nobody can be drawn: | ||
| * they are counted, not drawn as empty circles. | ||
| */ | ||
| export const NoUsernames: Story = { | ||
| args: { people: [{ _id: 'ada' }, { _id: 'grace' }], total: 4 }, | ||
| }; |
Uh oh!
There was an error while loading. Please reload this page.