Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,8 @@ When reviewing code (or writing code that will be reviewed):
- **Keep scope tight in design iterations** - When adjusting UI, avoid unrelated behavioral/SEO changes in the same commit unless explicitly requested.
- **Fix the calculation, not the collapse UX** - When a bug is a wrong value in a truncated/collapsed summary (e.g. per-company tenure on the collapsed profile), correct only the computation: derive the value from the complete dataset while rendering the exact same subset and keeping "Show More" visibility identical. Do NOT change the collapse unit (e.g. capping by positions vs by company groups) or the Show-More gating source as a side effect — those are visible UX shifts. Decouple "data summed/grouped over the full set" from "items rendered": compute over the full list, render the original truncated slice. Assert collapsed↔expanded value parity in a test.
- **Confirm target surface before implementing UI fixes** - If a bug report names a specific component or screen, update only that target unless expansion is explicitly requested.
- **The post composer has two audience/squad pickers — fix both** - Audience selection during post creation / link sharing is implemented twice: `AudienceChip` (`packages/shared/src/components/post/composer/AudienceChip.tsx`) inside `SmartComposerModal`, and `MultipleSourceSelect` (`packages/shared/src/components/post/write/MultipleSourceSelect.tsx`) on the `/squads/create` page. `CreatePostButton` (`packages/shared/src/components/post/write/CreatePostButton.tsx`) only opens the `SmartComposer` modal when `isLaptop && evaluateSmartComposer()` (the `featureSmartComposer` flag); on mobile/tablet, or when the experiment is off, it navigates to `/squads/create`, so a fix that only touches `AudienceChip` still shows the old behavior on mobile. When changing audience-picker UX, mirror it across both components (they intentionally share the same row-click-is-additive-toggle model).
- **For selectable rows, let the native `<label>`/checkbox handle the click — don't intercept it with a `role="button"` + `preventDefault`** - The shared `Checkbox` (`packages/shared/src/components/fields/Checkbox.tsx`) renders a real `<label htmlFor>` wrapping a hidden native `<input type="checkbox">`, so clicking anywhere on the row already toggles it and fires `onChange`. Do NOT wrap the row content in a focusable `role="button"` that calls `event.preventDefault()`/`stopPropagation()` to drive a custom click handler (as the old single-select `MultipleSourceSelect` did): that suppresses the label's native click-forwarding and, inside a Radix `Popover`/menu, the extra focusable element + prevented default interferes with the dismissable-layer/focus handling and can collapse the popover on touch. Wire selection through the checkbox's `onChange` and keep the row a plain label; it's simpler, accessible (native keyboard toggle), and stays open while selecting.
- **Keep action spacing consistent in control headers** - When adding icon/action buttons near search fields or other controls, match existing horizontal gaps on both sides to avoid controls touching each other.
- **Place feed promos in content flow unless explicitly sticky** - If a promo belongs between feed navigation and the feed list, render it in the feed/content layout so it pushes content down. Do not attach it to sticky nav with absolute positioning unless the requirement explicitly asks for overlay behavior.
- **Use `searchChildren` prop for content above feed results on search pages** - In `MainFeedLayout`, page `children` render AFTER the `<Feed>` component. To place banners/promos above feed results, pass them via the `searchChildren` prop (through `layoutProps` on the page component). Do NOT render them as page children — they will appear below all posts. Prefer reusing existing props over introducing new ones.
Expand Down
217 changes: 158 additions & 59 deletions packages/shared/src/components/post/composer/AudienceChip.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,42 +1,82 @@
import React from 'react';
import React, { useState } from 'react';
import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import type { Squad } from '../../../graphql/sources';
import { SourceMemberRole, SourceType } from '../../../graphql/sources';
import { AudienceChip } from './AudienceChip';

jest.mock('../../dropdown/DropdownMenu', () => ({
DropdownMenu: ({ children }: { children: React.ReactNode }) => (
<div>{children}</div>
),
DropdownMenuTrigger: ({ children }: { children: React.ReactNode }) =>
children,
DropdownMenuContent: ({ children }: { children: React.ReactNode }) => (
<div>{children}</div>
),
DropdownMenuItem: ({
children,
onSelect,
...props
}: {
children: React.ReactNode;
onSelect?: (event: Event) => void;
}) => (
<div
role="menuitem"
tabIndex={0}
onClick={(event) => onSelect?.(event.nativeEvent)}
onKeyDown={(event) => {
if (event.key === 'Enter' || event.key === ' ') {
onSelect?.(event.nativeEvent);
}
}}
{...props}
>
{children}
</div>
),
}));
// Mirror the Radix contract we rely on: content renders only while open, the
// trigger opens it, and selecting an item closes the menu UNLESS onSelect calls
// preventDefault(). This lets the tests actually assert the menu stays open.
jest.mock('../../dropdown/DropdownMenu', () => {
const { createContext, useContext, cloneElement } =
jest.requireActual<typeof React>('react');
const MenuContext = createContext<{
open: boolean;
setOpen: (v: boolean) => void;
}>({
open: false,
setOpen: () => {},
});
const select = (
onSelect: ((event: Event) => void) | undefined,
nativeEvent: Event,
setOpen: (v: boolean) => void,
) => {
onSelect?.(nativeEvent);
if (!nativeEvent.defaultPrevented) {
setOpen(false);
}
};
return {
DropdownMenu: ({
children,
open,
onOpenChange,
}: {
children: React.ReactNode;
open: boolean;
onOpenChange: (v: boolean) => void;
}) => (
<MenuContext.Provider value={{ open, setOpen: onOpenChange }}>
{children}
</MenuContext.Provider>
),
DropdownMenuTrigger: ({ children }: { children: React.ReactElement }) => {
const { setOpen } = useContext(MenuContext);
return cloneElement(children, { onClick: () => setOpen(true) });
},
DropdownMenuContent: ({ children }: { children: React.ReactNode }) => {
const { open } = useContext(MenuContext);
return open ? <div>{children}</div> : null;
},
DropdownMenuItem: ({
children,
onSelect,
...props
}: {
children: React.ReactNode;
onSelect?: (event: Event) => void;
}) => {
const { setOpen } = useContext(MenuContext);
return (
<div
role="menuitem"
tabIndex={0}
onClick={(event) => select(onSelect, event.nativeEvent, setOpen)}
onKeyDown={(event) => {
if (event.key === 'Enter' || event.key === ' ') {
select(onSelect, event.nativeEvent, setOpen);
}
}}
{...props}
>
{children}
</div>
);
},
};
});

const createAudience = (
id: string,
Expand Down Expand Up @@ -74,28 +114,55 @@ const audiences = [
designSquad,
];

const renderComponent = ({
// Owns selection state so the component behaves like the real controlled
// composer: each onChange feeds back into selectedIds and re-renders.
const StatefulHarness = ({
initialSelectedIds,
onChange,
}: {
initialSelectedIds: string[];
onChange: jest.Mock;
}) => {
const [selectedIds, setSelectedIds] = useState(initialSelectedIds);
return (
<AudienceChip
audiences={audiences}
selectedIds={selectedIds}
onChange={(ids) => {
onChange(ids);
setSelectedIds(ids);
}}
userAudienceId="user-1"
/>
);
};

// Renders the chip and opens the menu (the menu is closed until the trigger is
// clicked, matching Radix), returning the onChange spy and the open menu.
const openMenu = async ({
selectedIds = ['user-1'],
onChange = jest.fn(),
}: {
selectedIds?: string[];
onChange?: jest.Mock;
} = {}) => {
render(
<AudienceChip
audiences={audiences}
selectedIds={selectedIds}
onChange={onChange}
userAudienceId="user-1"
/>,
<StatefulHarness initialSelectedIds={selectedIds} onChange={onChange} />,
);

await userEvent.click(screen.getByRole('button', { name: /Posting to/ }));
return { onChange };
};

// The option row and the trigger can share a label (e.g. "Everyone"); the menu
// content renders after the trigger, so the option is the last match.
const clickOption = (label: string) => {
const matches = screen.getAllByText(label);
return userEvent.click(matches[matches.length - 1]);
};

describe('AudienceChip', () => {
it('renders checkbox controls for audience options', () => {
renderComponent();
it('renders checkbox controls for audience options', async () => {
await openMenu();

expect(
screen.getByRole('checkbox', {
Expand All @@ -109,18 +176,37 @@ describe('AudienceChip', () => {
).toBeInTheDocument();
});

it('single-selects an audience when clicking the row', async () => {
const { onChange } = renderComponent({
selectedIds: ['user-1', 'squad-backend'],
it('adds squads additively on row clicks and keeps the menu open', async () => {
const { onChange } = await openMenu();

await clickOption('Frontend');
expect(onChange).toHaveBeenLastCalledWith(['user-1', 'squad-frontend']);

await clickOption('Backend');
expect(onChange).toHaveBeenLastCalledWith([
'user-1',
'squad-frontend',
'squad-backend',
]);

// options are still mounted, so the menu stayed open across both clicks
expect(screen.getByText('Backend')).toBeInTheDocument();
expect(screen.getByText('3 audiences')).toBeInTheDocument();
});

it('deselects an already-selected squad on row click without closing', async () => {
const { onChange } = await openMenu({
selectedIds: ['user-1', 'squad-frontend'],
});

await userEvent.click(screen.getByText('Frontend'));
await clickOption('Frontend');

expect(onChange).toHaveBeenCalledWith(['squad-frontend']);
expect(onChange).toHaveBeenLastCalledWith(['user-1']);
expect(screen.getByText('Frontend')).toBeInTheDocument();
});

it('multi-selects an audience when clicking the checkbox', async () => {
const { onChange } = renderComponent();
it('multi-selects an audience when clicking the checkbox exactly once', async () => {
const { onChange } = await openMenu();

await userEvent.click(
screen.getByRole('checkbox', {
Expand All @@ -130,22 +216,35 @@ describe('AudienceChip', () => {

expect(onChange).toHaveBeenCalledTimes(1);
expect(onChange).toHaveBeenCalledWith(['user-1', 'squad-frontend']);
expect(screen.getByText('Frontend')).toBeInTheDocument();
});

it('keeps row single-select available when the multi-select checkbox is disabled', async () => {
const { onChange } = renderComponent({
it('enforces the 3-squad cap on row clicks', async () => {
const { onChange } = await openMenu({
selectedIds: ['squad-frontend', 'squad-backend', 'squad-mobile'],
});
const designCheckbox = screen.getByRole('checkbox', {
name: 'Toggle Design for multi-audience posting',
});

expect(designCheckbox).toBeDisabled();
expect(
screen.getByText('You can post to up to 3 squads'),
).toBeInTheDocument();

await userEvent.click(designCheckbox);
await clickOption('Design');
expect(onChange).not.toHaveBeenCalled();
});

it('falls back to Everyone when the last squad is removed via row click', async () => {
const { onChange } = await openMenu({ selectedIds: ['squad-frontend'] });

await clickOption('Frontend');

expect(onChange).toHaveBeenLastCalledWith(['user-1']);
});

it('keeps Everyone selected when it is the sole audience', async () => {
const { onChange } = await openMenu({ selectedIds: ['user-1'] });

await userEvent.click(screen.getByText('Design'));
expect(onChange).toHaveBeenCalledWith(['squad-design']);
await clickOption('Everyone');

expect(onChange).not.toHaveBeenCalled();
});
});
11 changes: 2 additions & 9 deletions packages/shared/src/components/post/composer/AudienceChip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -133,13 +133,6 @@ export const AudienceChip = ({
toggleSquad(option);
};

const selectSingleOption = (option: Squad) => {
if (!option.id) {
return;
}
onChange([option.id]);
};

const handleReset = () => {
if (userAudienceId) {
onChange([userAudienceId]);
Expand Down Expand Up @@ -223,11 +216,11 @@ export const AudienceChip = ({
<DropdownMenuItem
key={option.id}
onSelect={(event: Event) => {
event.preventDefault();
if (!option.id) {
event.preventDefault();
return;
}
selectSingleOption(option);
toggleOption(option);
}}
className="!h-9 gap-2 !overflow-visible !px-2"
>
Expand Down
Loading
Loading