Skip to content

Commit

Permalink
Issue 661 Tech Radar Typescript refactor (#1080)
Browse files Browse the repository at this point in the history
* Refactor RadarFooter

* Refactor RadarLegend Pt 1

* Refactor RadarLegend Pt 2

Seeing some errors that I don't fully understand around the proptypes
assertions seemingly being treated as null | undefined values.

It looks like I'll need to type Segment, which is not explicitly
in-scope for this issue.

* Fix segments regression

I incorrectly added type safety to the original implementation, causing
the segments to no longer be populated here. This change fixes that
regression while retaining the type safety.

This is all necessary because qidx and ridx are apparently both
nullable.

* Resolve remaining RadarLegend warnings

* Refactor RadarEntry

* Forcibly skip hooks to revert migration prettify

* Refactor RadarGrid

* Refactor RadarPlot & Best effort with PropTypes

A global search looks like no other plugins use PropTypes. I'm about to
remove them in the next commit because shapes don't seem to cooperate
with TypeScript, making the PropTypes either so vague that they're
mostly useless or specific enough to fail the build.

With that said, I wanted to get this version committed in case I need to
roll back to here. I wouldn't want to redo this work.

* Remove PropTypes from refactored components

The build now passes.

* Refactor RadarBubble

* Refactor Radar

* Move Radar functions out of component file

I could see these being split into separate files, but I think this is
enough for now.

* Small function signature change

* Align types

I'm not confident in this change. I'm very unclear about the reasoning
behind the type definitions in api.ts as they don't reflect the shape of
the values that I see when I print `data.entries` in the console.

The value of data.entries reflects a fully-populated Entry[], not a
RadarEntry[].

As such, I'm also not sure about the sample data. I would be more
hesitant to updated it, but the fact that the rest of the components
seem to be expecting that data makes it seem fairly clear that it must
be required to be in the updated shape.
  • Loading branch information
NWCalvank committed Jun 8, 2020
1 parent fb8e834 commit 4213098
Show file tree
Hide file tree
Showing 17 changed files with 831 additions and 803 deletions.
4 changes: 2 additions & 2 deletions plugins/tech-radar/src/api.ts
Expand Up @@ -35,8 +35,8 @@ export interface RadarEntry {
key: string; // react key
id: string;
moved: number;
quadrant: string;
ring: string;
quadrant: RadarQuadrant;
ring: RadarRing;
title: string;
url: string;
}
Expand Down
223 changes: 0 additions & 223 deletions plugins/tech-radar/src/components/Radar/Radar.jsx

This file was deleted.

4 changes: 2 additions & 2 deletions plugins/tech-radar/src/components/Radar/Radar.test.tsx
Expand Up @@ -31,9 +31,9 @@ const minProps = {
{
id: 'typescript',
title: 'TypeScript',
quadrant: 'languages',
quadrant: { id: 'languages', name: 'Languages' },
moved: 0,
ring: 'use',
ring: { id: 'use', name: 'USE', color: '#93c47d' },
url: '#',
},
],
Expand Down
60 changes: 60 additions & 0 deletions plugins/tech-radar/src/components/Radar/Radar.tsx
@@ -0,0 +1,60 @@
/*
* Copyright 2020 Spotify AB
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import React, { FC, useState, useRef } from 'react';
import RadarPlot from '../RadarPlot';
import { Ring, Quadrant, Entry } from '../../utils/types';
import { adjustQuadrants, adjustRings, adjustEntries } from './utils';

type Props = {
width: number;
height: number;
quadrants: Quadrant[];
rings: Ring[];
entries: Entry[];
svgProps?: object;
};

const Radar: FC<Props> = props => {
const { width, height, quadrants, rings, entries } = props;
const radius = Math.min(width, height) / 2;

const [activeEntry, setActiveEntry] = useState<Entry | null>();
const node = useRef<SVGSVGElement>(null);

// TODO(dflemstr): most of this can be heavily memoized if performance becomes a problem
adjustQuadrants(quadrants, radius, width, height);
adjustRings(rings, radius);
adjustEntries(entries, activeEntry, quadrants, rings, radius);

return (
<svg ref={node} width={width} height={height} {...props.svgProps}>
<RadarPlot
width={width}
height={height}
radius={radius}
entries={entries}
quadrants={quadrants}
rings={rings}
activeEntry={activeEntry || undefined}
onEntryMouseEnter={entry => setActiveEntry(entry)}
onEntryMouseLeave={() => setActiveEntry(null)}
/>
</svg>
);
};

export default Radar;

0 comments on commit 4213098

Please sign in to comment.