diff --git a/packages/shared/src/components/post/EndOfConversationShare.spec.tsx b/packages/shared/src/components/post/EndOfConversationShare.spec.tsx index c4220c49de..9282dee2dd 100644 --- a/packages/shared/src/components/post/EndOfConversationShare.spec.tsx +++ b/packages/shared/src/components/post/EndOfConversationShare.spec.tsx @@ -6,9 +6,9 @@ import { render, screen, waitFor, + within, } from '@testing-library/react'; import { QueryClient } from '@tanstack/react-query'; -import { GrowthBook } from '@growthbook/growthbook-react'; import { activeDiscussionCommentThreshold, EndOfConversationShare, @@ -47,11 +47,6 @@ const createPost = (numComments: number): Post => numComments, } as Post); -const enabledFlags = { - sharing_visibility: { defaultValue: true }, - share_end_of_conversation: { defaultValue: true }, -}; - beforeEach(() => { jest.clearAllMocks(); useViewSizeMock.mockReturnValue(true); // default: laptop @@ -63,16 +58,9 @@ beforeEach(() => { delete (navigator as unknown as { share?: unknown }).share; }); -const renderComponent = ( - numComments: number, - features: Record = enabledFlags, -): RenderResult => +const renderComponent = (numComments: number): RenderResult => render( - + , ); @@ -85,7 +73,7 @@ describe('EndOfConversationShare threshold', () => { expect(band()).not.toBeInTheDocument(); expect( - screen.queryByLabelText('Share this discussion'), + screen.queryByRole('button', { name: 'Copy link' }), ).not.toBeInTheDocument(); }); @@ -103,48 +91,62 @@ describe('EndOfConversationShare threshold', () => { }); }); -describe('EndOfConversationShare flags', () => { - it('stays hidden when the master kill-switch is off', () => { - renderComponent(12, { - sharing_visibility: { defaultValue: false }, - share_end_of_conversation: { defaultValue: true }, - }); +describe('EndOfConversationShare sharing', () => { + it('copies the link from the main half of the split button on desktop', async () => { + renderComponent(12); - expect(band()).not.toBeInTheDocument(); - }); + const copyButton = screen.getByRole('button', { name: 'Copy link' }); + // Both glyphs are always mounted so they can cross-fade; only the check's + // opacity says which one is showing. + const check = () => copyButton.querySelector('.text-status-success'); + expect(check()).toHaveClass('opacity-0'); - it('stays hidden when its own experiment flag is off', () => { - renderComponent(12, { - sharing_visibility: { defaultValue: true }, - share_end_of_conversation: { defaultValue: false }, + await act(async () => { + fireEvent.click(copyButton); }); - expect(band()).not.toBeInTheDocument(); - }); - - it('stays hidden with both flags at their defaults', () => { - renderComponent(12, {}); - - expect(band()).not.toBeInTheDocument(); + // The copy glyph cross-fades to a green check for the confirmation window. + await waitFor(() => expect(check()).not.toHaveClass('opacity-0')); + await waitFor(() => expect(writeText).toHaveBeenCalledWith(permalink)); + expect(displayToast).toHaveBeenCalledWith( + '✅ Copied link to clipboard', + expect.anything(), + ); + expect(logEvent).toHaveBeenCalledWith( + expect.objectContaining({ + event_name: LogEvent.SharePost, + extra: JSON.stringify({ + provider: ShareProvider.CopyLink, + origin: Origin.EndOfConversation, + }), + }), + ); }); -}); -describe('EndOfConversationShare sharing', () => { - it('copies the link and toasts on desktop', async () => { + it('opens the full share list from the chevron half, without copying', async () => { renderComponent(12); + // Radix dropdowns open on pointerdown (which jsdom cannot synthesise) or on + // Enter, so the keyboard path is what a test can drive. await act(async () => { - fireEvent.click(screen.getByLabelText('Share this discussion')); + fireEvent.keyDown(screen.getByLabelText('More share options'), { + key: 'Enter', + }); }); + + const popover = await screen.findByRole('menu'); + expect(within(popover).getByTestId('social-share-X')).toBeInTheDocument(); + expect( + within(popover).getByTestId('social-share-WhatsApp'), + ).toBeInTheDocument(); + expect(writeText).not.toHaveBeenCalled(); + + // The copy action inside the list still works and logs the same origin. await act(async () => { - fireEvent.click(await screen.findByText('Copy link')); + fireEvent.click(within(popover).getByTestId('social-share-Copy link')); }); await waitFor(() => expect(writeText).toHaveBeenCalledWith(permalink)); - expect(displayToast).toHaveBeenCalledWith( - '✅ Copied link to clipboard', - expect.anything(), - ); expect(logEvent).toHaveBeenCalledWith( expect.objectContaining({ event_name: LogEvent.SharePost, @@ -164,8 +166,12 @@ describe('EndOfConversationShare sharing', () => { renderComponent(12); + // Mobile keeps a single button — no chevron half, no popover. + expect( + screen.queryByLabelText('More share options'), + ).not.toBeInTheDocument(); await act(async () => { - fireEvent.click(screen.getByLabelText('Share this discussion')); + fireEvent.click(screen.getByRole('button', { name: 'Copy link' })); }); await waitFor(() => expect(share).toHaveBeenCalled()); diff --git a/packages/shared/src/components/post/EndOfConversationShare.tsx b/packages/shared/src/components/post/EndOfConversationShare.tsx index 4b148f7a1d..f3a4d4402b 100644 --- a/packages/shared/src/components/post/EndOfConversationShare.tsx +++ b/packages/shared/src/components/post/EndOfConversationShare.tsx @@ -9,9 +9,6 @@ import { TypographyType, } from '../typography/Typography'; import { ButtonSize, ButtonVariant } from '../buttons/common'; -import { useConditionalFeature } from '../../hooks/useConditionalFeature'; -import { useSharingVisibility } from '../../hooks/useSharingVisibility'; -import { featureShareEndOfConversation } from '../../lib/featureManagement'; import { useLogContext } from '../../contexts/LogContext'; import { postLogEvent } from '../../lib/feed'; import { LogEvent, Origin } from '../../lib/log'; @@ -26,21 +23,29 @@ import type { ShareProvider } from '../../lib/share'; */ export const activeDiscussionCommentThreshold = 3; -export const hasActiveDiscussion = (post: Post): boolean => +const hasActiveDiscussion = (post: Post): boolean => (post.numComments ?? 0) > activeDiscussionCommentThreshold; +export type EndOfConversationShareVariant = 'card' | 'flat'; + export interface EndOfConversationShareProps { post: Post; + /** + * `flat` (default) drops the fill and leans on a single hairline rule to + * separate the strip from the comments above it; `card` is the heavier + * self-contained surface. + */ + variant?: EndOfConversationShareVariant; className?: string; } /** - * The band itself, including the activity threshold but without the feature - * gates, so Storybook can render both sides of the threshold without a - * GrowthBook instance. + * Encouraging share band rendered below the comment list of an active + * discussion. Ships to everyone — the comment threshold is the only condition. */ -export const EndOfConversationShareBand = ({ +export const EndOfConversationShare = ({ post, + variant = 'flat', className, }: EndOfConversationShareProps): ReactElement | null => { const { logEvent } = useLogContext(); @@ -59,9 +64,13 @@ export const EndOfConversationShareBand = ({ return ( ); }; - -/** - * Encouraging share band rendered below the comment list of an active - * discussion. Gated by the initiative kill-switch plus its own experiment flag, - * and only evaluated once the post is past the activity threshold. - */ -export const EndOfConversationShare = ({ - post, - className, -}: EndOfConversationShareProps): ReactElement | null => { - const isActive = hasActiveDiscussion(post); - const { isEnabled: isInitiativeEnabled } = useSharingVisibility(isActive); - const { value: isBandEnabled } = useConditionalFeature({ - feature: featureShareEndOfConversation, - shouldEvaluate: isActive && isInitiativeEnabled, - }); - - if (!isInitiativeEnabled || !isBandEnabled) { - return null; - } - - return ; -}; diff --git a/packages/shared/src/components/post/PostComments.spec.tsx b/packages/shared/src/components/post/PostComments.spec.tsx index 06235481f8..d6b9f0c15d 100644 --- a/packages/shared/src/components/post/PostComments.spec.tsx +++ b/packages/shared/src/components/post/PostComments.spec.tsx @@ -2,7 +2,6 @@ import React from 'react'; import type { RenderResult } from '@testing-library/react'; import { render, screen, waitFor } from '@testing-library/react'; import { QueryClient } from '@tanstack/react-query'; -import { GrowthBook } from '@growthbook/growthbook-react'; import { PostComments } from './PostComments'; import type { Post } from '../../graphql/posts'; import { TestBootProvider } from '../../../__tests__/helpers/boot'; @@ -39,28 +38,17 @@ const createPost = (numComments: number): Post => numComments, } as Post); -const enabledFlags = { - sharing_visibility: { defaultValue: true }, - share_end_of_conversation: { defaultValue: true }, -}; - beforeEach(() => { jest.clearAllMocks(); }); -const renderComponent = ( - numComments: number, - features: Record = {}, -): RenderResult => { +const renderComponent = (numComments: number): RenderResult => { mockRequestMethod.mockReturnValue(() => Promise.resolve(buildComments(numComments)), ); return render( - + { it('appends the band after the last comment on an active discussion', async () => { - renderComponent(6, enabledFlags); + renderComponent(6); const comments = await screen.findAllByTestId('main-comment'); await waitFor(() => expect(comments).toHaveLength(6)); @@ -82,18 +70,8 @@ describe('PostComments end-of-conversation share band', () => { ); }); - it('leaves the comment list untouched when the flags are off', async () => { - renderComponent(6); - - const comments = await screen.findAllByTestId('main-comment'); - await waitFor(() => expect(comments).toHaveLength(6)); - - expect(screen.queryByRole('complementary')).not.toBeInTheDocument(); - expect(comments[5].nextElementSibling).toBeNull(); - }); - - it('keeps the band hidden on a quiet discussion even with the flags on', async () => { - renderComponent(2, enabledFlags); + it('keeps the band hidden on a quiet discussion', async () => { + renderComponent(2); const comments = await screen.findAllByTestId('main-comment'); await waitFor(() => expect(comments).toHaveLength(2)); diff --git a/packages/shared/src/components/post/PostComments.tsx b/packages/shared/src/components/post/PostComments.tsx index 42137bde59..1a04deab0a 100644 --- a/packages/shared/src/components/post/PostComments.tsx +++ b/packages/shared/src/components/post/PostComments.tsx @@ -176,8 +176,9 @@ export function PostComments({ {!hideEndOfConversationShare && ( )} diff --git a/packages/shared/src/components/share/CopyStateIcon.tsx b/packages/shared/src/components/share/CopyStateIcon.tsx new file mode 100644 index 0000000000..6c5defa913 --- /dev/null +++ b/packages/shared/src/components/share/CopyStateIcon.tsx @@ -0,0 +1,47 @@ +import type { ReactElement } from 'react'; +import React from 'react'; +import classNames from 'classnames'; +import { CopyIcon, VIcon } from '../icons'; +import type { IconProps } from '../Icon'; + +/** + * easeOutExpo — the curve the design-system dropdown animates on. It + * decelerates into the target with no overshoot, which is what keeps a swap + * from reading as a wobble. + */ +export const EASE_OUT_EXPO = 'ease-[cubic-bezier(0.16,1,0.3,1)]'; + +/** + * A copy is a rare, deliberate moment, so the confirmation earns real motion. + * Both glyphs share one grid cell so the label never shifts mid-swap, and the + * transition collapses to an instant swap under `prefers-reduced-motion`. + */ +export const CopyStateIcon = ({ + copied, + className, + ...props +}: IconProps & { copied: boolean }): ReactElement => { + const layer = classNames( + className, + 'col-start-1 row-start-1 transition-[opacity,transform,filter] duration-200 motion-reduce:transition-none', + EASE_OUT_EXPO, + ); + + return ( + + + + + ); +}; diff --git a/packages/shared/src/components/share/ShareActions.tsx b/packages/shared/src/components/share/ShareActions.tsx index 50ee53e30e..a375ea9caf 100644 --- a/packages/shared/src/components/share/ShareActions.tsx +++ b/packages/shared/src/components/share/ShareActions.tsx @@ -13,8 +13,10 @@ import { useShareOrCopyLink } from '../../hooks/useShareOrCopyLink'; import { shouldUseNativeShare } from '../../lib/func'; import { ShareProvider } from '../../lib/share'; import type { ReferralCampaignKey } from '../../lib/referral'; +import { CopyStateIcon } from './CopyStateIcon'; +import { SplitShareButton } from './SplitShareButton'; -export type ShareActionsVariant = 'icon' | 'inline'; +export type ShareActionsVariant = 'icon' | 'inline' | 'split'; export interface ShareActionsProps { link: string; @@ -28,6 +30,13 @@ export interface ShareActionsProps { buttonSize?: ButtonSize; /** Tooltip + accessible label for the icon-only trigger. */ label?: string; + /** + * Render `triggerText` beside the icon instead of an icon-only trigger. Gated + * so existing icon-only consumers keep their exact DOM. + */ + triggerText?: string; + /** `split` variant only: label for the chevron half that opens the list. */ + dropdownLabel?: string; emailTitle?: string; emailSummary?: string; className?: string; @@ -46,6 +55,8 @@ export function ShareActions({ buttonVariant = ButtonVariant.Tertiary, buttonSize = ButtonSize.Small, label = 'Copy link', + triggerText, + dropdownLabel = 'More share options', emailTitle, emailSummary, className, @@ -56,6 +67,22 @@ export function ShareActions({ const [copying, shareOrCopy] = useShareOrCopyLink({ link, text, cid }); const closeTimeout = useRef>(); + const onCopy = () => { + onShare?.(ShareProvider.CopyLink); + shareOrCopy(); + }; + + // `copying` stays true for a second after a copy, which is the whole window + // for the confirmation. The green-check swap is scoped to the split control — + // icon-only triggers keep the existing `secondary` fill so this does not + // restyle every share surface in the app. + const copyIcon = + variant === 'split' ? ( + + ) : ( + + ); + const list = ( { - onShare?.(ShareProvider.CopyLink); - shareOrCopy(); - }} + onCopy={onCopy} onNativeShare={() => { onShare?.(ShareProvider.Native); shareOrCopy(); @@ -92,7 +116,7 @@ export function ShareActions({ type="button" variant={buttonVariant} size={buttonSize} - icon={} + icon={copyIcon} aria-label={label} className={className} onClick={() => { @@ -103,11 +127,31 @@ export function ShareActions({ ); shareOrCopy(); }} - /> + > + {triggerText} + ); } + if (variant === 'split') { + return ( + + ); + } + const cancelClose = () => { if (closeTimeout.current) { clearTimeout(closeTimeout.current); @@ -136,12 +180,14 @@ export function ShareActions({ type="button" variant={buttonVariant} size={buttonSize} - icon={} + icon={copyIcon} aria-label={label} pressed={open} className={className} {...hoverProps} - /> + > + {triggerText} + void; + variant?: ButtonVariant; + size?: ButtonSize; + className?: string; +} + +/** + * The halves meet at a single hairline, so the standard side paddings would + * leave a canyon around it. Both sides tighten by one step from the DS value — + * the outer edges keep the standard padding, so the control still reads as one + * button. + */ +const MAIN_INNER_PADDING: Record = { + [ButtonSize.XLarge]: '!pr-5', + [ButtonSize.Large]: '!pr-4', + [ButtonSize.Medium]: '!pr-3', + [ButtonSize.Small]: '!pr-2', + [ButtonSize.XSmall]: '!pr-1.5', +}; + +/** Drops the icon-only square so the chevron hugs the seam symmetrically. */ +const CHEVRON_PADDING: Record = { + [ButtonSize.XLarge]: '!w-auto !px-3', + [ButtonSize.Large]: '!w-auto !px-2.5', + [ButtonSize.Medium]: '!w-auto !px-2', + [ButtonSize.Small]: '!w-auto !px-1.5', + [ButtonSize.XSmall]: '!w-auto !px-1', +}; + +const DIVIDER_BASE = + "relative border-l-0 before:absolute before:left-0 before:w-px before:content-['']"; + +/** + * Variants that paint `--button-default-border-color: transparent` have no + * border for the divider to match, so they draw their own 1px rule. Every other + * variant returns nothing and keeps its real border as the divider. + * + * Alpha is mixed into the colour rather than applied as a separate utility: + * `before:opacity-*` does not survive this project's Tailwind build on + * pseudo-elements and silently renders at full strength. `bg-current` is + * likewise unusable — the theme replaces Tailwind's `colors` wholesale and has + * no `current` key, so it compiles to nothing at all. + */ +const dividerFor = (variant: ButtonVariant): string | false => { + // Sits on a solid fill, where only the label colour is guaranteed to read. + if (variant === ButtonVariant.Primary) { + return classNames( + DIVIDER_BASE, + 'before:inset-y-0 before:bg-[color-mix(in_srgb,var(--button-color,var(--button-default-color)),transparent_80%)]', + ); + } + + // A bare ghost button: a full-height rule would float with nothing to anchor + // it, so it gets a shorter one in the colour `tailwind/buttons.ts` gives the + // Subtle variant's border. + if (variant === ButtonVariant.Tertiary) { + return classNames( + DIVIDER_BASE, + 'before:inset-y-1.5 before:bg-[color-mix(in_srgb,var(--theme-border-subtlest-primary),transparent_70%)]', + ); + } + + return false; +}; + +/** + * Two real buttons that read as one control: the left half runs the primary + * action, the right half drops the standard menu. Geometry matches a standard + * button at every size — only the shared edge deviates. + */ +export const SplitShareButton = ({ + label, + dropdownLabel, + triggerText, + menu, + copied, + onCopy, + variant = ButtonVariant.Tertiary, + size = ButtonSize.Small, + className, +}: SplitShareButtonProps): ReactElement => { + const [open, setOpen] = useState(false); + + return ( +
+ + + + + +
+ ); +}; diff --git a/packages/shared/src/lib/featureManagement.ts b/packages/shared/src/lib/featureManagement.ts index f585086aff..97b38cbdd2 100644 --- a/packages/shared/src/lib/featureManagement.ts +++ b/packages/shared/src/lib/featureManagement.ts @@ -308,12 +308,3 @@ export const featureSharingVisibility = new Feature( // high-traffic icon, so it ramps on its own flag to watch share/copy metrics. // Keep the default `false` (control = existing `LinkIcon`). export const featureShareCopyIcon = new Feature('share_copy_icon', false); - -// Renders an encouraging share band below the comment list once a post has an -// active discussion (more than 3 comments). Part of the sharing-visibility -// initiative, so it also sits behind the `sharing_visibility` kill-switch. -// Keep the default `false` — GrowthBook ramps it. -export const featureShareEndOfConversation = new Feature( - 'share_end_of_conversation', - false, -); diff --git a/packages/storybook/stories/components/EndOfConversationShare.stories.tsx b/packages/storybook/stories/components/EndOfConversationShare.stories.tsx index 525fa35437..9fe6748606 100644 --- a/packages/storybook/stories/components/EndOfConversationShare.stories.tsx +++ b/packages/storybook/stories/components/EndOfConversationShare.stories.tsx @@ -1,15 +1,21 @@ import type { Meta, StoryObj } from '@storybook/react-vite'; import React from 'react'; -import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; import { activeDiscussionCommentThreshold, - EndOfConversationShareBand, + EndOfConversationShare, } from '@dailydotdev/shared/src/components/post/EndOfConversationShare'; import type { Post } from '@dailydotdev/shared/src/graphql/posts'; -import { getLogContextStatic } from '@dailydotdev/shared/src/contexts/LogContext'; -import AuthContext from '@dailydotdev/shared/src/contexts/AuthContext'; -import type { LoggedUser } from '@dailydotdev/shared/src/lib/user'; -import { fn } from 'storybook/test'; +import { ShareActions } from '@dailydotdev/shared/src/components/share/ShareActions'; +import { + ButtonSize, + ButtonVariant, +} from '@dailydotdev/shared/src/components/buttons/common'; +import { + Typography, + TypographyColor, + TypographyType, +} from '@dailydotdev/shared/src/components/typography/Typography'; +import { shareDecorator } from './share.mocks'; const basePost = { id: 'p1', @@ -22,91 +28,40 @@ const basePost = { const withPost = (numComments: number): Post => ({ ...basePost, numComments } as Post); -const meta: Meta = { +const meta: Meta = { title: 'Components/Share/EndOfConversationShare', - component: EndOfConversationShareBand, + component: EndOfConversationShare, args: { post: basePost }, parameters: { docs: { description: { component: `Share band rendered below the comment list. It only appears once a post has an active discussion — more than ${activeDiscussionCommentThreshold} comments, read from -\`Post.numComments\`. In the app it is additionally gated by the \`sharing_visibility\` -kill-switch and the \`share_end_of_conversation\` experiment flag.`, +\`Post.numComments\`. It ships to everyone — the +comment threshold is the only condition.`, }, }, }, decorators: [ - (Story) => { - const queryClient = new QueryClient({ - defaultOptions: { queries: { retry: false, staleTime: Infinity } }, - }); - // Mock the short-URL resolution so copy/social actions don't hit network. - queryClient.setQueryData(['shortUrl'], 'https://dly.to/abc123'); - - const LogContext = getLogContextStatic(); - - const mockUser = { - id: '1', - name: 'Test User', - username: 'testuser', - email: 'test@example.com', - image: - 'https://daily-now-res.cloudinary.com/image/upload/placeholder.jpg', - providers: ['google'], - createdAt: '2024-01-01T00:00:00.000Z', - permalink: 'https://daily.dev/testuser', - } as unknown as LoggedUser; - - return ( - - - false, - }} - > -
- -
-
-
-
- ); - }, + shareDecorator('flex min-h-40 w-full max-w-2xl flex-col justify-center'), ], }; export default meta; -type Story = StoryObj; +type Story = StoryObj; -// Active discussion: the band renders below the comment list. +// Active discussion in the default flat treatment: no fill, no rounding, just +// a hairline rule above the strip. export const ActiveDiscussion: Story = { args: { post: withPost(12) }, }; +// The heavier alternative: a self-contained float surface with a full border. +export const Card: Story = { + args: { post: withPost(12), variant: 'card' }, +}; + // Exactly at the threshold — still hidden, the band needs MORE than 3 comments. export const AtThreshold: Story = { args: { post: withPost(activeDiscussionCommentThreshold) }, @@ -122,16 +77,252 @@ export const NoComments: Story = { args: { post: withPost(0) }, }; -// Narrow the Storybook viewport to see the mobile layout: the band stacks and -// a single tap on the share button opens the native share sheet (falling back -// to copy when the device has no share target). +const Section = ({ + title, + note, + children, +}: { + title: string; + note: string; + children: React.ReactNode; +}) => ( +
+
+ + {title} + + + {note} + +
+ {children} +
+); + +// A hidden state renders nothing at all, so the outline marks where the band +// would have been — otherwise the gallery just shows a gap. +const Hidden = ({ post }: { post: Post }) => ( +
+ + Nothing renders ({post.numComments ?? 0} comments) + + +
+); + +const commentAuthors = ['Ido Shamun', 'Nimrod Kramer', 'Tsahi Matsliah']; + +// Stand-in for the comment list so the band can be judged against what sits +// above it, without dragging the real Comment component's context in. +const CommentSkeletons = () => ( + <> + {commentAuthors.map((author) => ( +
+
+
+ + {author} + +
+
+
+
+ ))} + +); + +/** + * Rendered at a real phone viewport, not a narrow box on a desktop viewport — + * the `tablet:` breakpoint reads the viewport, so only a true 390px frame shows + * the stacked layout and the chevron-less single tap. `AllStates` embeds this + * story in a 390px iframe for exactly that reason. + */ export const Mobile: Story = { - args: { post: withPost(12) }, - decorators: [ - (Story) => ( -
- + render: () => ( +
+
+ +
- ), - ], + +
+ ), +}; + +// Storybook renders each story in its own iframe, so embedding one here gives +// the band a genuine 390px viewport instead of a squeezed desktop layout. The +// theme global is mirrored so the frame follows the toolbar toggle. +const MobileFrame = () => { + const [isDark, setIsDark] = React.useState(true); + + React.useEffect(() => { + const read = () => + setIsDark(document.documentElement.classList.contains('dark')); + read(); + + const observer = new MutationObserver(read); + observer.observe(document.documentElement, { + attributes: true, + attributeFilter: ['class'], + }); + + return () => observer.disconnect(); + }, []); + + return ( +