Browse Source

feat(issues): Display stacktrace links as icons (#62894)

Scott Cooper 1 year ago
parent
commit
0abd154c50

+ 16 - 12
static/app/components/events/interfaces/frame/stacktraceLink.spec.tsx

@@ -145,11 +145,9 @@ describe('StacktraceLink', function () {
     render(<StacktraceLink frame={frame} event={event} line="foo()" />, {
       context: RouterContextFixture(),
     });
-    expect(await screen.findByRole('link')).toHaveAttribute(
-      'href',
-      'https://something.io#L233'
-    );
-    expect(screen.getByText('Open this line in GitHub')).toBeInTheDocument();
+    const link = await screen.findByRole('link', {name: 'Open this line in GitHub'});
+    expect(link).toBeInTheDocument();
+    expect(link).toHaveAttribute('href', 'https://something.io#L233');
   });
 
   it('displays fix modal on error', async function () {
@@ -234,12 +232,14 @@ describe('StacktraceLink', function () {
       organization,
     });
 
-    expect(await screen.findByText('Open in Codecov')).toHaveAttribute(
+    const link = await screen.findByRole('link', {name: 'Open in Codecov'});
+    expect(link).toBeInTheDocument();
+    expect(link).toHaveAttribute(
       'href',
       'https://app.codecov.io/gh/path/to/file.py#L233'
     );
 
-    await userEvent.click(await screen.findByText('Open in Codecov'));
+    await userEvent.click(link);
     expect(analyticsSpy).toHaveBeenCalledWith(
       'integrations.stacktrace_codecov_link_clicked',
       expect.anything()
@@ -318,11 +318,12 @@ describe('StacktraceLink', function () {
         context: RouterContextFixture(),
       }
     );
-    expect(await screen.findByRole('link')).toHaveAttribute(
+    const link = await screen.findByRole('link', {name: 'Open this line in GitHub'});
+    expect(link).toBeInTheDocument();
+    expect(link).toHaveAttribute(
       'href',
       'https://www.github.com/username/path/to/file.py#L100'
     );
-    expect(screen.getByText('Open this line in GitHub')).toBeInTheDocument();
   });
 
   it('renders the link using sourceUrl instead of sourceLink if it exists for a .NET project', async function () {
@@ -349,11 +350,12 @@ describe('StacktraceLink', function () {
         context: RouterContextFixture(),
       }
     );
-    expect(await screen.findByRole('link')).toHaveAttribute(
+    const link = await screen.findByRole('link', {name: 'Open this line in GitHub'});
+    expect(link).toBeInTheDocument();
+    expect(link).toHaveAttribute(
       'href',
       'https://www.github.com/url/from/code/mapping#L1'
     );
-    expect(screen.getByText('Open this line in GitHub')).toBeInTheDocument();
   });
 
   it('hides stacktrace link if there is no source link for .NET projects', async function () {
@@ -401,7 +403,9 @@ describe('StacktraceLink', function () {
       'https://something.io#L233'
     );
 
-    expect(await screen.getByText('GitHub')).toBeInTheDocument();
+    expect(
+      screen.getByRole('link', {name: 'Open this line in GitHub'})
+    ).toBeInTheDocument();
 
     jest.useRealTimers();
   });

+ 31 - 8
static/app/components/events/interfaces/frame/stacktraceLink.tsx

@@ -15,6 +15,7 @@ import HookOrDefault from 'sentry/components/hookOrDefault';
 import ExternalLink from 'sentry/components/links/externalLink';
 import Link from 'sentry/components/links/link';
 import Placeholder from 'sentry/components/placeholder';
+import {Tooltip} from 'sentry/components/tooltip';
 import {IconClose, IconWarning} from 'sentry/icons';
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
@@ -172,9 +173,20 @@ function CodecovLink({
     organization?.features?.includes('issue-details-stacktrace-link-in-frame') ?? false;
 
   return (
-    <OpenInLink href={coverageUrl} openInNewTab onClick={onOpenCodecovLink}>
-      <StyledIconWrapper>{getIntegrationIcon('codecov', 'sm')}</StyledIconWrapper>
-      {hasStacktraceLinkFeatureFlag ? t('Codecov') : t('Open in Codecov')}
+    <OpenInLink
+      href={coverageUrl}
+      openInNewTab
+      onClick={onOpenCodecovLink}
+      aria-label={hasStacktraceLinkFeatureFlag ? t('Open in Codecov') : undefined}
+    >
+      <Tooltip
+        title={t('Open in Codecov')}
+        disabled={!hasStacktraceLinkFeatureFlag}
+        skipWrapper
+      >
+        <StyledIconWrapper>{getIntegrationIcon('codecov', 'sm')}</StyledIconWrapper>
+      </Tooltip>
+      {hasStacktraceLinkFeatureFlag ? null : t('Open in Codecov')}
     </OpenInLink>
   );
 }
@@ -290,7 +302,7 @@ export function StacktraceLink({frame, event, line}: StacktraceLinkProps) {
       <StacktraceLinkWrapper>
         <Placeholder
           height={hasStacktraceLinkFeatureFlag ? '14px' : '24px'}
-          width={hasStacktraceLinkFeatureFlag ? '171px' : '120px'}
+          width={hasStacktraceLinkFeatureFlag ? '40px' : '120px'}
         />
       </StacktraceLinkWrapper>
     );
@@ -308,12 +320,23 @@ export function StacktraceLink({frame, event, line}: StacktraceLinkProps) {
             frame.lineNo
           )}
           openInNewTab
+          aria-label={
+            hasStacktraceLinkFeatureFlag
+              ? t('Open this line in %s', match.config.provider.name)
+              : undefined
+          }
         >
-          <StyledIconWrapper>
-            {getIntegrationIcon(match.config.provider.key, 'sm')}
-          </StyledIconWrapper>
+          <Tooltip
+            disabled={!hasStacktraceLinkFeatureFlag}
+            title={t('Open this line in %s', match.config.provider.name)}
+            skipWrapper
+          >
+            <StyledIconWrapper>
+              {getIntegrationIcon(match.config.provider.key, 'sm')}
+            </StyledIconWrapper>
+          </Tooltip>
           {hasStacktraceLinkFeatureFlag
-            ? match.config.provider.name
+            ? null
             : t('Open this line in %s', match.config.provider.name)}
         </OpenInLink>
         {shouldShowCodecovFeatures(organization, match) ? (