chore(autofix): Move feedback button (#82062)

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). <img width="673" alt="Screenshot 2024-12-12 at 3 07 24 PM" src="https://github.com/user-attachments/assets/2b4cba50-53f9-40a9-aecb-9aa9e67e22df" />
getsentry · Dec 12, 2024 · a78b8a9 · a78b8a9
1 parent c5bdc3a
commit a78b8a9
Show file tree

Hide file tree

Showing 6 changed files with 69 additions and 46 deletions.
diff --git 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 (
-    <Button
+    <StyledButton
       ref={buttonRef}
-      size="xs"
+      size="zero"
+      borderless
       icon={<IconMegaphone />}
       onClick={() =>
         openForm({
@@ -29,8 +31,16 @@ function AutofixFeedback() {
       }
     >
       {t('Give Feedback')}
-    </Button>
+    </StyledButton>
   );
 }
 
+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
@@ -126,7 +126,7 @@ describe('AutofixMessageBox Analytics', () => {
 
     render(<AutofixMessageBox {...changesStepProps} />);
 
-    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(<AutofixMessageBox {...changesStepProps} />);
 
-    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
@@ -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(<AutofixMessageBox {...changesStepProps} />);
 
-    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(<AutofixMessageBox {...changesStepProps} />);
 
+    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(<AutofixMessageBox {...changesStepProps} />);
 
-    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(<AutofixMessageBox {...multipleChangesProps} />);
 
-    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(<AutofixMessageBox {...changesStepProps} />);
 
-    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,26 +338,26 @@ describe('AutofixMessageBox', () => {
     ).toBeInTheDocument();
   });
 
-  it('shows segmented control with "Add tests" option for changes step', () => {
+  it('shows segmented control options for changes step', () => {
     render(<AutofixMessageBox {...changesStepProps} />);
 
-    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(<AutofixMessageBox {...changesStepProps} />);
 
-    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?')
     ).toBeInTheDocument();
     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(<AutofixMessageBox {...changesStepProps} />);
 
-    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
@@ -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({
           <ContentArea>
             {step && (
               <StepHeader>
+                <StepIconContainer>
+                  <StepIcon step={step} />
+                </StepIconContainer>
                 <StepTitle
                   dangerouslySetInnerHTML={{
                     __html: singleLineRenderer(step.title),
@@ -425,9 +429,7 @@ function AutofixMessageBox({
                       </AnimatePresence>
                     </ScrollIntoViewButtonWrapper>
                   )}
-                  <StepIconContainer>
-                    <StepIcon step={step} />
-                  </StepIconContainer>
+                  <AutofixFeedback />
                 </StepHeaderRightSection>
               </StepHeader>
             )}
@@ -464,14 +466,14 @@ function AutofixMessageBox({
                     onChange={setChangesMode}
                     aria-label={t('Changes selection')}
                   >
+                    <SegmentedControl.Item key="create_prs">
+                      {t('Approve')}
+                    </SegmentedControl.Item>
                     <SegmentedControl.Item key="give_feedback">
-                      {t('Give feedback')}
+                      {t('Iterate')}
                     </SegmentedControl.Item>
                     <SegmentedControl.Item key="add_tests">
-                      {t('Add tests')}
-                    </SegmentedControl.Item>
-                    <SegmentedControl.Item key="create_prs">
-                      {t('Approve changes')}
+                      {t('Test')}
                     </SegmentedControl.Item>
                   </SegmentedControl>
                 </Fragment>
@@ -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
@@ -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(<AutofixSteps {...propsWithChanges} />);
 
+    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
@@ -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
             />
           </HeaderContainer>
           {autofixData && (
-            <ButtonBar gap={1}>
-              <AutofixFeedback />
-              <Button
-                size="xs"
-                onClick={reset}
-                title={
-                  autofixData.created_at
-                    ? `Last run at ${autofixData.created_at.split('T')[0]}`
-                    : null
-                }
-              >
-                {t('Start Over')}
-              </Button>
-            </ButtonBar>
+            <Button
+              size="xs"
+              onClick={reset}
+              title={
+                autofixData.created_at
+                  ? `Last run at ${autofixData.created_at.split('T')[0]}`
+                  : null
+              }
+            >
+              {t('Start Over')}
+            </Button>
           )}
         </HeaderText>
         {aiConfig.isAutofixSetupLoading ? (