From b56815b401af0337074d9eebb6c84f7e57cada97 Mon Sep 17 00:00:00 2001
From: Niall McCullagh <niallmccullagh@users.noreply.github.com>
Date: Tue, 9 Mar 2021 23:20:28 +0000
Subject: [PATCH] Fix issue where radar entry dialog with external url

If an entry has both a radar description and an external url, when
a user clicks on the blip the description dialog would briefly
appear before they are redirected to the specified url.

Expected behaviour is not to automatically redirect the user so that
they can read the dialog and choose to follow the link for more details.

Signed-off-by: Niall McCullagh <mccullagh_niall@hotmail.com>
Signed-off-by: Niall McCullagh <niallmccullagh@users.noreply.github.com>
---
 .changeset/cyan-rings-live.md                    |  5 +++++
 .../components/RadarEntry/RadarEntry.test.tsx    | 16 ++++++++++++++++
 .../src/components/RadarEntry/RadarEntry.tsx     |  3 ++-
 plugins/tech-radar/src/sampleData.ts             |  4 +++-
 4 files changed, 26 insertions(+), 2 deletions(-)
 create mode 100644 .changeset/cyan-rings-live.md

diff --git a/.changeset/cyan-rings-live.md b/.changeset/cyan-rings-live.md
new file mode 100644
index 0000000000..3b6d2b6d9b
--- /dev/null
+++ b/.changeset/cyan-rings-live.md
@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-tech-radar': patch
+---
+
+Fixes issue where radar description dialog is not shown when the entry has an url external to the radar page
diff --git a/plugins/tech-radar/src/components/RadarEntry/RadarEntry.test.tsx b/plugins/tech-radar/src/components/RadarEntry/RadarEntry.test.tsx
index 0424624007..8cadd33da4 100644
--- a/plugins/tech-radar/src/components/RadarEntry/RadarEntry.test.tsx
+++ b/plugins/tech-radar/src/components/RadarEntry/RadarEntry.test.tsx
@@ -79,4 +79,20 @@ describe('RadarEntry', () => {
     expect(radarDescription).toBeInTheDocument();
     expect(screen.getByText(String(minProps.value))).toBeInTheDocument();
   });
+
+  it('should render blip with url equal to # if description present', () => {
+    const withUrl = {
+      ...optionalProps,
+      url: 'http://backstage.io',
+    };
+    render(
+      <ThemeProvider theme={lightTheme}>
+        <svg>
+          <RadarEntry {...withUrl} />
+        </svg>
+      </ThemeProvider>,
+    );
+
+    expect(screen.getByRole('button')).toHaveAttribute('href', '#');
+  });
 });
diff --git a/plugins/tech-radar/src/components/RadarEntry/RadarEntry.tsx b/plugins/tech-radar/src/components/RadarEntry/RadarEntry.tsx
index fb47a75c71..c3500a8c2d 100644
--- a/plugins/tech-radar/src/components/RadarEntry/RadarEntry.tsx
+++ b/plugins/tech-radar/src/components/RadarEntry/RadarEntry.tsx
@@ -111,11 +111,12 @@ const RadarEntry = (props: Props): JSX.Element => {
         />
       )}
       {description ? (
+        // eslint-disable-next-line jsx-a11y/anchor-is-valid
         <a
           className={classes.link}
           onClick={handleClickOpen}
-          href={url ? url : '#'}
           role="button"
+          href="#"
           tabIndex={0}
           onKeyPress={toggle}
         >
diff --git a/plugins/tech-radar/src/sampleData.ts b/plugins/tech-radar/src/sampleData.ts
index 613e7b3cdc..309c3f6b88 100644
--- a/plugins/tech-radar/src/sampleData.ts
+++ b/plugins/tech-radar/src/sampleData.ts
@@ -72,9 +72,11 @@ entries.push({
       moved: 1,
       ringId: 'use',
       date: new Date('2020-08-06'),
+      description:
+        'Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur',
     },
   ],
-  url: '#',
+  url: 'https://webpack.js.org/',
   key: 'webpack',
   id: 'webpack',
   title: 'Webpack',