diff --git a/src/components/FeedbackDialog.test.tsx b/src/components/FeedbackDialog.test.tsx index 571af88a..5becf3aa 100644 --- a/src/components/FeedbackDialog.test.tsx +++ b/src/components/FeedbackDialog.test.tsx @@ -2,10 +2,11 @@ import { fireEvent, render, screen, waitFor } from '@testing-library/react' import { beforeEach, describe, expect, it, vi } from 'vitest' import { FeedbackDialog } from './FeedbackDialog' import { - TOLARIA_CANNY_URL, TOLARIA_GITHUB_CONTRIBUTING_URL, TOLARIA_GITHUB_DISCUSSIONS_URL, TOLARIA_GITHUB_ISSUES_URL, + TOLARIA_GITHUB_PULL_REQUESTS_URL, + TOLARIA_PRODUCT_BOARD_URL, } from '../constants/feedback' import { APP_COMMAND_EVENT_NAME, APP_COMMAND_IDS } from '../hooks/appCommandDispatcher' import { rememberFeedbackDialogOpener } from '../lib/feedbackDialogOpener' @@ -33,15 +34,20 @@ describe('FeedbackDialog', () => { render() expect(screen.getByTestId('feedback-dialog')).toBeInTheDocument() expect(screen.getByText('Contribute to Tolaria')).toBeInTheDocument() - expect(screen.getByText('Feature request / improvement idea')).toBeInTheDocument() - expect(screen.getByText('Community / discussion')).toBeInTheDocument() + expect(screen.getByText('Pick the path that fits what you want to do! Any type of help is appreciated')).toBeInTheDocument() + expect(screen.getByText('Feature requests')).toBeInTheDocument() + expect(screen.getByText('Discussions')).toBeInTheDocument() expect(screen.getByText('Contribute code')).toBeInTheDocument() expect(screen.getByText('Report a bug')).toBeInTheDocument() + expect(screen.getByText(/Search on the product board first/i)).toBeInTheDocument() + expect(screen.getByText(/Use GitHub Discussions for/i)).toBeInTheDocument() + expect(screen.getByText(/Attach the sanitized diagnostic bundle please!/i)).toBeInTheDocument() + expect(screen.queryByText(/Sanitized and optional/i)).not.toBeInTheDocument() }) it('focuses the primary CTA when opened', async () => { render() - const cta = screen.getByRole('button', { name: 'Open Canny' }) + const cta = screen.getByRole('button', { name: 'Open Product Board' }) await waitFor(() => expect(cta).toHaveFocus()) }) @@ -49,15 +55,17 @@ describe('FeedbackDialog', () => { const onClose = vi.fn() render() - fireEvent.click(screen.getByRole('button', { name: 'Open Canny' })) + fireEvent.click(screen.getByRole('button', { name: 'Open Product Board' })) fireEvent.click(screen.getByRole('button', { name: 'Open Discussions' })) + fireEvent.click(screen.getByRole('button', { name: 'Open Pull Requests' })) fireEvent.click(screen.getByRole('button', { name: 'Open Contributing Guide' })) fireEvent.click(screen.getByRole('button', { name: 'Open GitHub Issues' })) - await waitFor(() => expect(openExternalUrl).toHaveBeenNthCalledWith(1, TOLARIA_CANNY_URL)) + await waitFor(() => expect(openExternalUrl).toHaveBeenNthCalledWith(1, TOLARIA_PRODUCT_BOARD_URL)) expect(openExternalUrl).toHaveBeenNthCalledWith(2, TOLARIA_GITHUB_DISCUSSIONS_URL) - expect(openExternalUrl).toHaveBeenNthCalledWith(3, TOLARIA_GITHUB_CONTRIBUTING_URL) - expect(openExternalUrl).toHaveBeenNthCalledWith(4, TOLARIA_GITHUB_ISSUES_URL) + expect(openExternalUrl).toHaveBeenNthCalledWith(3, TOLARIA_GITHUB_PULL_REQUESTS_URL) + expect(openExternalUrl).toHaveBeenNthCalledWith(4, TOLARIA_GITHUB_CONTRIBUTING_URL) + expect(openExternalUrl).toHaveBeenNthCalledWith(5, TOLARIA_GITHUB_ISSUES_URL) expect(onClose).not.toHaveBeenCalled() expect(screen.getByTestId('feedback-dialog')).toBeInTheDocument() }) @@ -71,7 +79,7 @@ describe('FeedbackDialog', () => { render() - fireEvent.click(screen.getByRole('button', { name: 'Copy diagnostics' })) + fireEvent.click(screen.getByRole('button', { name: 'Copy sanitized diagnostics' })) await waitFor(() => expect(writeText).toHaveBeenCalledTimes(1)) expect(writeText.mock.calls[0]?.[0]).toContain('Tolaria sanitized diagnostics') @@ -84,10 +92,10 @@ describe('FeedbackDialog', () => { openExternalUrl.mockRejectedValueOnce(new Error('blocked')) render() - fireEvent.click(screen.getByRole('button', { name: 'Open Canny' })) + fireEvent.click(screen.getByRole('button', { name: 'Open Product Board' })) - expect(await screen.findByText(/couldn’t open Canny automatically/i)).toBeInTheDocument() - expect(screen.getByText(TOLARIA_CANNY_URL)).toBeInTheDocument() + expect(await screen.findByText(/couldn’t open Product Board automatically/i)).toBeInTheDocument() + expect(screen.getByText(TOLARIA_PRODUCT_BOARD_URL)).toBeInTheDocument() }) it('closes when pressing Escape', () => { @@ -97,7 +105,7 @@ describe('FeedbackDialog', () => { expect(onClose).toHaveBeenCalledOnce() }) - it('closes when clicking Close', () => { + it('closes when clicking the top-right Close control', () => { const onClose = vi.fn() render() fireEvent.click(screen.getByRole('button', { name: 'Close' })) diff --git a/src/components/FeedbackDialog.tsx b/src/components/FeedbackDialog.tsx index 99e34ef9..1a68a29d 100644 --- a/src/components/FeedbackDialog.tsx +++ b/src/components/FeedbackDialog.tsx @@ -14,20 +14,21 @@ import { Dialog, DialogContent, DialogDescription, - DialogFooter, DialogHeader, DialogTitle, } from '@/components/ui/dialog' import { - TOLARIA_CANNY_URL, TOLARIA_GITHUB_CONTRIBUTING_URL, TOLARIA_GITHUB_DISCUSSIONS_URL, TOLARIA_GITHUB_ISSUES_URL, + TOLARIA_GITHUB_PULL_REQUESTS_URL, + TOLARIA_PRODUCT_BOARD_URL, } from '../constants/feedback' import { buildSanitizedDiagnosticBundle, startFeedbackDiagnosticsCapture, } from '../lib/feedbackDiagnostics' +import { cn } from '../lib/utils' import { takeFeedbackDialogOpener } from '../lib/feedbackDialogOpener' import { useBuildNumber } from '../hooks/useBuildNumber' import { APP_COMMAND_EVENT_NAME, APP_COMMAND_IDS } from '../hooks/appCommandDispatcher' @@ -45,6 +46,7 @@ interface ContributionCardProps { description: string ctaLabel: string icon: typeof Lightbulb + tone: ContributionTone onAction: () => void autoFocus?: boolean secondaryAction?: ReactNode @@ -62,6 +64,14 @@ interface ContributionPath { label: string url: string icon: typeof Lightbulb + tone: ContributionTone + secondaryLink?: ContributionLink +} + +interface ContributionLink { + ctaLabel: string + label: string + url: string } const EMPTY_DIALOG_OPENER: ReturnType = { @@ -69,38 +79,94 @@ const EMPTY_DIALOG_OPENER: ReturnType = { reopenCommandPalette: false, } +type ContributionTone = 'green' | 'yellow' | 'purple' | 'red' + +const CONTRIBUTION_TONE_CLASSES: Record = { + green: 'bg-[var(--accent-green-light)] text-[var(--accent-green)]', + yellow: 'bg-[var(--accent-yellow-light)] text-[var(--accent-yellow)]', + purple: 'bg-[var(--accent-purple-light)] text-[var(--accent-purple)]', + red: 'bg-[var(--accent-red-light)] text-[var(--accent-red)]', +} + +const CONTRIBUTION_BUTTON_CLASSES: Record = { + green: 'border-[var(--accent-green)] hover:bg-[var(--accent-green-light)] [&_svg]:text-[var(--accent-green)]', + yellow: 'border-[var(--accent-yellow)] hover:bg-[var(--accent-yellow-light)] [&_svg]:text-[var(--accent-yellow)]', + purple: 'border-[var(--accent-purple)] hover:bg-[var(--accent-purple-light)] [&_svg]:text-[var(--accent-purple)]', + red: 'border-[var(--accent-red)] hover:bg-[var(--accent-red-light)] [&_svg]:text-[var(--accent-red)]', +} + const CONTRIBUTION_PATHS: ContributionPath[] = [ { - title: 'Feature request / improvement idea', - description: 'Search Canny first, upvote an existing idea if it is already there, and only create a new post when it is genuinely new.', - ctaLabel: 'Open Canny', - label: 'Canny', - url: TOLARIA_CANNY_URL, + title: 'Feature requests', + description: 'Search on the product board first, upvote an existing idea if already there, and only create a new post when genuinely new!', + ctaLabel: 'Open Product Board', + label: 'Product Board', + url: TOLARIA_PRODUCT_BOARD_URL, icon: Lightbulb, + tone: 'green', }, { - title: 'Community / discussion', - description: 'Use Discussions for questions, broader conversations, idea sharing, and community context that is not a concrete bug report.', + title: 'Discussions', + description: 'Use GitHub Discussions for questions, broader conversations, idea sharing, and community context that is not a concrete bug report.', ctaLabel: 'Open Discussions', label: 'GitHub Discussions', url: TOLARIA_GITHUB_DISCUSSIONS_URL, icon: MessagesSquare, + tone: 'purple', }, { title: 'Contribute code', description: 'Small, focused pull requests are welcome. Check planned or in-progress work first so you are building in the right place.', - ctaLabel: 'Open Contributing Guide', - label: 'the contributing guide', - url: TOLARIA_GITHUB_CONTRIBUTING_URL, + ctaLabel: 'Open Pull Requests', + label: 'GitHub Pull Requests', + url: TOLARIA_GITHUB_PULL_REQUESTS_URL, icon: GitPullRequest, + tone: 'yellow', + secondaryLink: { + ctaLabel: 'Open Contributing Guide', + label: 'the contributing guide', + url: TOLARIA_GITHUB_CONTRIBUTING_URL, + }, }, ] +function ContributionLinkButton({ + label, + tone, + onAction, + autoFocus = false, + accented = true, +}: { + label: string + tone: ContributionTone + onAction: () => void + autoFocus?: boolean + accented?: boolean +}) { + return ( + + ) +} + function ContributionCard({ title, description, ctaLabel, icon: Icon, + tone, onAction, autoFocus = false, secondaryAction, @@ -109,7 +175,7 @@ function ContributionCard({
- + {title} @@ -119,10 +185,7 @@ function ContributionCard({ - + {secondaryAction ? {secondaryAction} : null} @@ -150,6 +213,10 @@ function LinkFallbackBanner({ linkFallback }: { linkFallback: LinkFallback | nul ) } +function getCopyDiagnosticsLabel(copyState: 'idle' | 'copied' | 'failed') { + return copyState === 'copied' ? 'Diagnostics copied' : 'Copy sanitized diagnostics' +} + function BugReportActions({ copyState, canCopyDiagnostics, @@ -168,12 +235,9 @@ function BugReportActions({ onClick={onCopyDiagnostics} disabled={!canCopyDiagnostics} > - {copyState === 'copied' ? 'Diagnostics copied' : 'Copy diagnostics'} + {getCopyDiagnosticsLabel(copyState)} {copyState === 'copied' ? : } -

- Sanitized and optional. Paths and token-like strings are redacted by default. -

{copyState === 'copied' ? (

Diagnostics copied.

) : null} @@ -273,22 +337,36 @@ function ContributionGrid({ }) { return (
- {CONTRIBUTION_PATHS.map((path, index) => ( - onOpenLink(path.label, path.url)} - /> - ))} + {CONTRIBUTION_PATHS.map((path, index) => { + const secondaryLink = path.secondaryLink + + return ( + onOpenLink(path.label, path.url)} + secondaryAction={secondaryLink ? ( + onOpenLink(secondaryLink.label, secondaryLink.url)} + /> + ) : undefined} + /> + ) + })} onOpenLink('GitHub Issues', TOLARIA_GITHUB_ISSUES_URL)} secondaryAction={( { if (!next) handleClose() }}> - + Contribute to Tolaria - Pick the path that fits what you want to do. The links stay focused, the diagnostics are sanitized, - and you can work through the whole flow with the keyboard. + Pick the path that fits what you want to do! Any type of help is appreciated @@ -352,12 +429,6 @@ export function FeedbackDialog({ canCopyDiagnostics={canCopyDiagnostics} onCopyDiagnostics={handleCopyDiagnostics} /> - - - - ) diff --git a/src/constants/feedback.ts b/src/constants/feedback.ts index 20a68de7..ef667d3a 100644 --- a/src/constants/feedback.ts +++ b/src/constants/feedback.ts @@ -1,4 +1,5 @@ -export const TOLARIA_CANNY_URL = 'https://tolaria.canny.io/' +export const TOLARIA_PRODUCT_BOARD_URL = 'https://tolaria.canny.io/' export const TOLARIA_GITHUB_DISCUSSIONS_URL = 'https://github.com/refactoringhq/tolaria/discussions' export const TOLARIA_GITHUB_CONTRIBUTING_URL = 'https://github.com/refactoringhq/tolaria/blob/main/CONTRIBUTING.md' export const TOLARIA_GITHUB_ISSUES_URL = 'https://github.com/refactoringhq/tolaria/issues' +export const TOLARIA_GITHUB_PULL_REQUESTS_URL = 'https://github.com/refactoringhq/tolaria/pulls' diff --git a/tests/smoke/contribute-modal.spec.ts b/tests/smoke/contribute-modal.spec.ts index 6683658e..70992a18 100644 --- a/tests/smoke/contribute-modal.spec.ts +++ b/tests/smoke/contribute-modal.spec.ts @@ -39,7 +39,7 @@ test.describe('Contribute modal', () => { await expect(page.getByTestId('feedback-dialog')).toBeVisible() await expect(page.getByRole('heading', { name: 'Contribute to Tolaria' })).toBeVisible() - await expect(page.getByRole('button', { name: 'Open Canny' })).toBeFocused() + await expect(page.getByRole('button', { name: 'Open Product Board' })).toBeFocused() await page.keyboard.press('Enter') await expect.poll(async () => page.evaluate(() => (window as typeof window & { __tolariaOpenedUrls: string[] }).__tolariaOpenedUrls)).toContain('https://tolaria.canny.io/') @@ -50,8 +50,13 @@ test.describe('Contribute modal', () => { await expect.poll(async () => page.evaluate(() => (window as typeof window & { __tolariaOpenedUrls: string[] }).__tolariaOpenedUrls)).toContain('https://github.com/refactoringhq/tolaria/discussions') await page.keyboard.press('Tab') - await expect(page.getByRole('button', { name: 'Open Contributing Guide' })).toBeFocused() + await expect(page.getByRole('button', { name: 'Open Pull Requests' })).toBeFocused() await page.keyboard.press('Enter') + await expect.poll(async () => page.evaluate(() => (window as typeof window & { __tolariaOpenedUrls: string[] }).__tolariaOpenedUrls)).toContain('https://github.com/refactoringhq/tolaria/pulls') + + await page.keyboard.press('Tab') + await expect(page.getByRole('button', { name: 'Open Contributing Guide' })).toBeFocused() + await page.keyboard.press('Space') await expect.poll(async () => page.evaluate(() => (window as typeof window & { __tolariaOpenedUrls: string[] }).__tolariaOpenedUrls)).toContain('https://github.com/refactoringhq/tolaria/blob/main/CONTRIBUTING.md') await page.keyboard.press('Tab') @@ -60,7 +65,7 @@ test.describe('Contribute modal', () => { await expect.poll(async () => page.evaluate(() => (window as typeof window & { __tolariaOpenedUrls: string[] }).__tolariaOpenedUrls)).toContain('https://github.com/refactoringhq/tolaria/issues') await page.keyboard.press('Tab') - await expect(page.getByRole('button', { name: 'Copy diagnostics' })).toBeFocused() + await expect(page.getByRole('button', { name: 'Copy sanitized diagnostics' })).toBeFocused() await page.keyboard.press('Space') await expect.poll(async () => page.evaluate(() => (window as typeof window & { __tolariaCopiedBundles: string[] }).__tolariaCopiedBundles.length)).toBe(1)