From a78b8a99f46667917550488436bfb0d08938daf1 Mon Sep 17 00:00:00 2001 From: Rohan Agarwal <47861399+roaga@users.noreply.github.com> Date: Thu, 12 Dec 2024 18:21:46 -0500 Subject: [PATCH] chore(autofix): Move feedback button (#82062) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moves feedback button out of the header and into the message box. Also renames tabs for clarity (note that we'll rework the segmented control pattern soon). Screenshot 2024-12-12 at 3 07 24 PM --- .../events/autofix/autofixFeedback.tsx | 16 +++++-- .../autofixMessageBox.analytics.spec.tsx | 4 +- .../events/autofix/autofixMessageBox.spec.tsx | 42 ++++++++++++------- .../events/autofix/autofixMessageBox.tsx | 21 ++++++---- .../events/autofix/autofixSteps.spec.tsx | 5 ++- .../streamline/sidebar/solutionsHubDrawer.tsx | 27 +++++------- 6 files changed, 69 insertions(+), 46 deletions(-) diff --git a/static/app/components/events/autofix/autofixFeedback.tsx b/static/app/components/events/autofix/autofixFeedback.tsx index 1d6010fed3b065..32e9bdc2445ea8 100644 --- a/static/app/components/events/autofix/autofixFeedback.tsx +++ b/static/app/components/events/autofix/autofixFeedback.tsx @@ -1,4 +1,5 @@ import {useRef} from 'react'; +import styled from '@emotion/styled'; import {Button} from 'sentry/components/button'; import {IconMegaphone} from 'sentry/icons/iconMegaphone'; @@ -14,9 +15,10 @@ function AutofixFeedback() { } return ( - + ); } +const StyledButton = styled(Button)` + padding: 0; + margin: 0; + font-size: ${p => p.theme.fontSizeSmall}; + font-weight: ${p => p.theme.fontWeightNormal}; + color: ${p => p.theme.subText}; +`; + export default AutofixFeedback; diff --git a/static/app/components/events/autofix/autofixMessageBox.analytics.spec.tsx b/static/app/components/events/autofix/autofixMessageBox.analytics.spec.tsx index 54d1ed27006d71..66ca4db3bf4bb4 100644 --- a/static/app/components/events/autofix/autofixMessageBox.analytics.spec.tsx +++ b/static/app/components/events/autofix/autofixMessageBox.analytics.spec.tsx @@ -126,7 +126,7 @@ describe('AutofixMessageBox Analytics', () => { render(); - await userEvent.click(screen.getByRole('radio', {name: 'Approve changes'})); + await userEvent.click(screen.getByRole('radio', {name: 'Approve'})); // Find the last call to Button that matches our Create PR button const createPRButtonCall = mockButton.mock.calls.find( @@ -156,7 +156,7 @@ describe('AutofixMessageBox Analytics', () => { render(); - await userEvent.click(screen.getByRole('radio', {name: 'Approve changes'})); + await userEvent.click(screen.getByRole('radio', {name: 'Approve'})); // Find the last call to Button that matches our Setup button const setupButtonCall = mockButton.mock.calls.find( diff --git a/static/app/components/events/autofix/autofixMessageBox.spec.tsx b/static/app/components/events/autofix/autofixMessageBox.spec.tsx index 1954cd41683551..d6e3a0cb1d75e3 100644 --- a/static/app/components/events/autofix/autofixMessageBox.spec.tsx +++ b/static/app/components/events/autofix/autofixMessageBox.spec.tsx @@ -74,6 +74,15 @@ describe('AutofixMessageBox', () => { (addSuccessMessage as jest.Mock).mockClear(); (addErrorMessage as jest.Mock).mockClear(); MockApiClient.clearMockResponses(); + + MockApiClient.addMockResponse({ + url: '/issues/123/autofix/setup/?check_write_access=true', + method: 'GET', + body: { + genAIConsent: {ok: true}, + integration: {ok: true}, + }, + }); }); it('renders correctly with default props', () => { @@ -191,20 +200,23 @@ describe('AutofixMessageBox', () => { it('renders segmented control for changes step', () => { render(); - expect(screen.getByRole('radio', {name: 'Give feedback'})).toBeInTheDocument(); - expect(screen.getByRole('radio', {name: 'Approve changes'})).toBeInTheDocument(); + expect(screen.getByRole('radio', {name: 'Iterate'})).toBeInTheDocument(); + expect(screen.getByRole('radio', {name: 'Approve'})).toBeInTheDocument(); + expect(screen.getByRole('radio', {name: 'Test'})).toBeInTheDocument(); }); - it('shows feedback input when "Give feedback" is selected', () => { + it('shows feedback input when "Iterate" is selected', async () => { render(); + await userEvent.click(screen.getByRole('radio', {name: 'Iterate'})); + expect( screen.getByPlaceholderText('Share helpful context or feedback...') ).toBeInTheDocument(); expect(screen.getByRole('button', {name: 'Send'})).toBeInTheDocument(); }); - it('shows "Create PR" button when "Approve changes" is selected', async () => { + it('shows "Create PR" button when "Approve" is selected', async () => { MockApiClient.addMockResponse({ url: '/issues/123/autofix/setup/?check_write_access=true', method: 'GET', @@ -219,7 +231,7 @@ describe('AutofixMessageBox', () => { render(); - await userEvent.click(screen.getByRole('radio', {name: 'Approve changes'})); + await userEvent.click(screen.getByRole('radio', {name: 'Approve'})); expect( screen.getByText('Draft 1 pull request for the above changes?') @@ -250,7 +262,7 @@ describe('AutofixMessageBox', () => { render(); - await userEvent.click(screen.getByRole('radio', {name: 'Approve changes'})); + await userEvent.click(screen.getByRole('radio', {name: 'Approve'})); expect( screen.getByText('Draft 2 pull requests for the above changes?') @@ -308,7 +320,7 @@ describe('AutofixMessageBox', () => { render(); - await userEvent.click(screen.getByRole('radio', {name: 'Approve changes'})); + await userEvent.click(screen.getByRole('radio', {name: 'Approve'})); expect( screen.getByText('Draft 1 pull request for the above changes?') @@ -326,18 +338,18 @@ describe('AutofixMessageBox', () => { ).toBeInTheDocument(); }); - it('shows segmented control with "Add tests" option for changes step', () => { + it('shows segmented control options for changes step', () => { render(); - expect(screen.getByRole('radio', {name: 'Give feedback'})).toBeInTheDocument(); - expect(screen.getByRole('radio', {name: 'Add tests'})).toBeInTheDocument(); - expect(screen.getByRole('radio', {name: 'Approve changes'})).toBeInTheDocument(); + expect(screen.getByRole('radio', {name: 'Approve'})).toBeInTheDocument(); + expect(screen.getByRole('radio', {name: 'Iterate'})).toBeInTheDocument(); + expect(screen.getByRole('radio', {name: 'Test'})).toBeInTheDocument(); }); - it('shows "Add Tests" button and static message when "Add tests" is selected', async () => { + it('shows "Test" button and static message when "Test" is selected', async () => { render(); - await userEvent.click(screen.getByRole('radio', {name: 'Add tests'})); + await userEvent.click(screen.getByRole('radio', {name: 'Test'})); expect( screen.getByText('Write unit tests to make sure the issue is fixed?') @@ -345,7 +357,7 @@ describe('AutofixMessageBox', () => { expect(screen.getByRole('button', {name: 'Add Tests'})).toBeInTheDocument(); }); - it('sends correct message when "Add Tests" is clicked without onSend prop', async () => { + it('sends correct message when "Test" is clicked without onSend prop', async () => { MockApiClient.addMockResponse({ method: 'POST', url: '/issues/123/autofix/update/', @@ -354,7 +366,7 @@ describe('AutofixMessageBox', () => { render(); - await userEvent.click(screen.getByRole('radio', {name: 'Add tests'})); + await userEvent.click(screen.getByRole('radio', {name: 'Test'})); await userEvent.click(screen.getByRole('button', {name: 'Add Tests'})); await waitFor(() => { diff --git a/static/app/components/events/autofix/autofixMessageBox.tsx b/static/app/components/events/autofix/autofixMessageBox.tsx index 62ac0ca81ad643..0963d0dd0be641 100644 --- a/static/app/components/events/autofix/autofixMessageBox.tsx +++ b/static/app/components/events/autofix/autofixMessageBox.tsx @@ -5,6 +5,7 @@ import {AnimatePresence, type AnimationProps, motion} from 'framer-motion'; import {addErrorMessage, addSuccessMessage} from 'sentry/actionCreators/indicator'; import {openModal} from 'sentry/actionCreators/modal'; import {Button, LinkButton} from 'sentry/components/button'; +import AutofixFeedback from 'sentry/components/events/autofix/autofixFeedback'; import {AutofixSetupWriteAccessModal} from 'sentry/components/events/autofix/autofixSetupWriteAccessModal'; import { type AutofixCodebaseChange, @@ -343,7 +344,7 @@ function AutofixMessageBox({ const [changesMode, setChangesMode] = useState< 'give_feedback' | 'add_tests' | 'create_prs' - >('give_feedback'); + >('create_prs'); const changes = isChangesStep && step?.type === AutofixStepType.CHANGES ? step.changes : []; @@ -402,6 +403,9 @@ function AutofixMessageBox({ {step && ( + + + )} - - - + )} @@ -464,14 +466,14 @@ function AutofixMessageBox({ onChange={setChangesMode} aria-label={t('Changes selection')} > + + {t('Approve')} + - {t('Give feedback')} + {t('Iterate')} - {t('Add tests')} - - - {t('Approve changes')} + {t('Test')} @@ -618,6 +620,7 @@ const StepHeader = styled('div')` padding: 0 ${space(1)} ${space(1)} ${space(1)}; font-size: ${p => p.theme.fontSizeMedium}; font-family: ${p => p.theme.text.family}; + gap: ${space(1)}; `; const InputArea = styled('div')` diff --git a/static/app/components/events/autofix/autofixSteps.spec.tsx b/static/app/components/events/autofix/autofixSteps.spec.tsx index 18b0067e8007a5..2778cab5882455 100644 --- a/static/app/components/events/autofix/autofixSteps.spec.tsx +++ b/static/app/components/events/autofix/autofixSteps.spec.tsx @@ -150,7 +150,8 @@ describe('AutofixSteps', () => { it('handles iterating on changes step', async () => { MockApiClient.addMockResponse({ - url: '/issues/group1/autofix/setup/', + url: '/issues/group1/autofix/setup/?check_write_access=true', + method: 'GET', body: { genAIConsent: {ok: true}, integration: {ok: true}, @@ -194,6 +195,8 @@ describe('AutofixSteps', () => { render(); + await userEvent.click(screen.getByRole('radio', {name: 'Iterate'})); + const input = screen.getByPlaceholderText('Share helpful context or feedback...'); await userEvent.type(input, 'Feedback on changes'); await userEvent.click(screen.getByRole('button', {name: 'Send'})); diff --git a/static/app/views/issueDetails/streamline/sidebar/solutionsHubDrawer.tsx b/static/app/views/issueDetails/streamline/sidebar/solutionsHubDrawer.tsx index 632a8d8579edd3..21a2194c08e640 100644 --- a/static/app/views/issueDetails/streamline/sidebar/solutionsHubDrawer.tsx +++ b/static/app/views/issueDetails/streamline/sidebar/solutionsHubDrawer.tsx @@ -8,8 +8,6 @@ import ProjectAvatar from 'sentry/components/avatar/projectAvatar'; import FeatureBadge from 'sentry/components/badge/featureBadge'; import {Breadcrumbs as NavigationBreadcrumbs} from 'sentry/components/breadcrumbs'; import {Button} from 'sentry/components/button'; -import ButtonBar from 'sentry/components/buttonBar'; -import AutofixFeedback from 'sentry/components/events/autofix/autofixFeedback'; import {AutofixSetupContent} from 'sentry/components/events/autofix/autofixSetupModal'; import {AutofixSteps} from 'sentry/components/events/autofix/autofixSteps'; import {useAiAutofix} from 'sentry/components/events/autofix/useAutofix'; @@ -194,20 +192,17 @@ export function SolutionsHubDrawer({group, project, event}: SolutionsHubDrawerPr /> {autofixData && ( - - - - + )} {aiConfig.isAutofixSetupLoading ? (