Browse Source

ref(autofix): Refactor UI so the icons are on the left again (#66644)

Move the Autofix step icons back to the left side + bring back the green
checkmark

The step icons were hard to see and in CI tools which we are basing this
UX off of, developers should be more familiar with icons being on the
left (see github actions on this PR for example) and with checkmarks
marking steps as done.

Also hides the chevron arrow on the right when theres nothing to expand.

<img width="1464" alt="Screenshot 2024-03-08 at 3 01 10 PM"
src="https://github.com/getsentry/sentry/assets/30991498/c8853f57-ab56-457d-84dd-5369cd514faa">
Jenn Mueng 1 year ago
parent
commit
369d11a01e

+ 33 - 16
static/app/components/events/autofix/autofixSteps.tsx

@@ -10,7 +10,7 @@ import type {
 } from 'sentry/components/events/autofix/types';
 import LoadingIndicator from 'sentry/components/loadingIndicator';
 import Panel from 'sentry/components/panels/panel';
-import {IconChevron, IconClose, IconFatal} from 'sentry/icons';
+import {IconCheckmark, IconChevron, IconClose, IconFatal} from 'sentry/icons';
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
 
@@ -27,6 +27,7 @@ function StepIcon({status}: StepIconProps) {
     case 'ERROR':
       return <IconFatal size="sm" color="red300" />;
     case 'COMPLETED':
+      return <IconCheckmark size="sm" color="green300" isCircled />;
     default:
       return null;
   }
@@ -65,7 +66,7 @@ function Progress({progress}: {progress: AutofixProgressItem | AutofixStep}) {
   );
 }
 
-export function Step({step, isChild, stepNumber}: StepProps) {
+export function Step({step, isChild}: StepProps) {
   const isActive = step.status !== 'PENDING' && step.status !== 'CANCELLED';
   const [isExpanded, setIsExpanded] = useState(false);
 
@@ -85,25 +86,25 @@ export function Step({step, isChild, stepNumber}: StepProps) {
           }
         }}
       >
-        <StepTitle>
-          {stepNumber ? `${stepNumber}. ` : null}
-          {step.title}
-        </StepTitle>
-        {activeLog && !isExpanded && (
-          <StepHeaderDescription>{activeLog}</StepHeaderDescription>
-        )}
+        <StepHeaderLeft>
+          <StepIconContainer>
+            <StepIcon status={step.status} />
+          </StepIconContainer>
+          <StepTitle>{step.title}</StepTitle>
+          {activeLog && !isExpanded && (
+            <StepHeaderDescription>{activeLog}</StepHeaderDescription>
+          )}
+        </StepHeaderLeft>
         <StepHeaderRight>
-          <StepIcon status={step.status} />
-          {isActive && (
+          {isActive && hasContent ? (
             <Button
               icon={<IconChevron size="xs" direction={isExpanded ? 'down' : 'right'} />}
               aria-label={t('Toggle step details')}
               aria-expanded={isExpanded}
               size="zero"
               borderless
-              disabled={!hasContent}
             />
-          )}
+          ) : null}
         </StepHeaderRight>
       </StepHeader>
       {isExpanded && (
@@ -142,9 +143,8 @@ const StepCard = styled(Panel)<{active?: boolean}>`
 `;
 
 const StepHeader = styled('div')<{isActive: boolean; isChild?: boolean}>`
-  display: grid;
+  display: flex;
   justify-content: space-between;
-  grid-template-columns: auto 1fr auto;
   align-items: center;
   padding: ${space(2)};
   font-size: ${p => p.theme.fontSizeMedium};
@@ -156,24 +156,41 @@ const StepHeader = styled('div')<{isActive: boolean; isChild?: boolean}>`
   }
 `;
 
+const StepHeaderLeft = styled('div')`
+  display: flex;
+  align-items: center;
+  flex: 1;
+`;
+
 const StepHeaderDescription = styled('div')`
   font-size: ${p => p.theme.fontSizeSmall};
   color: ${p => p.theme.subText};
   padding: 0 ${space(2)} 0 ${space(1)};
   margin-left: ${space(1)};
   border-left: 1px solid ${p => p.theme.border};
+  flex-grow: 1;
   ${p => p.theme.overflowEllipsis};
 `;
 
+const StepIconContainer = styled('div')`
+  display: flex;
+  align-items: center;
+  margin-right: ${space(1)};
+`;
+
 const StepHeaderRight = styled('div')`
   display: flex;
   align-items: center;
   gap: ${space(1)};
-  grid-column: -1;
 `;
 
 const StepTitle = styled('div')`
   font-weight: bold;
+  white-space: nowrap;
+  display: flex;
+  flex-shrink: 1;
+  align-items: center;
+  flex-grow: 0;
 
   span {
     margin-right: ${space(1)};

+ 1 - 1
static/app/components/events/autofix/index.spec.tsx

@@ -129,7 +129,7 @@ describe('AiAutofix', () => {
 
     await userEvent.click(screen.getByRole('button', {name: 'Toggle log details'}));
 
-    expect(screen.getByText('1. I am processing')).toBeInTheDocument();
+    expect(screen.getByText('I am processing')).toBeInTheDocument();
     expect(screen.getByText('oh yes I am')).toBeInTheDocument();
   });
 });