From de1fe3c0e84f0be71b9d9ea8cf5a693ac7048749 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20DOREAU?= Date: Fri, 19 Jun 2020 00:47:25 +0200 Subject: [PATCH] refactor(tech-radar): style + code quality --- .../tech-radar/src/components/Radar/Radar.tsx | 6 +- .../tech-radar/src/components/Radar/utils.ts | 2 +- .../components/RadarBubble/RadarBubble.tsx | 4 +- .../src/components/RadarComponent.tsx | 28 ++--- .../src/components/RadarEntry/RadarEntry.tsx | 47 ++++---- .../components/RadarFooter/RadarFooter.tsx | 4 +- .../components/RadarLegend/RadarLegend.tsx | 103 +++++++++--------- .../tech-radar/src/components/RadarPage.tsx | 4 +- .../src/components/RadarPlot/RadarPlot.tsx | 6 +- plugins/tech-radar/src/utils/components.tsx | 35 ++++++ 10 files changed, 134 insertions(+), 105 deletions(-) create mode 100644 plugins/tech-radar/src/utils/components.tsx diff --git a/plugins/tech-radar/src/components/Radar/Radar.tsx b/plugins/tech-radar/src/components/Radar/Radar.tsx index 1db655730d..a1e4d2da78 100644 --- a/plugins/tech-radar/src/components/Radar/Radar.tsx +++ b/plugins/tech-radar/src/components/Radar/Radar.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import React, { FC, useState, useRef } from 'react'; +import React, { useState, useRef } from 'react'; import RadarPlot from '../RadarPlot'; import { Ring, Quadrant, Entry } from '../../utils/types'; import { adjustQuadrants, adjustRings, adjustEntries } from './utils'; @@ -28,7 +28,7 @@ type Props = { svgProps?: object; }; -const Radar: FC = props => { +const Radar = (props: Props): JSX.Element => { const { width, height, quadrants, rings, entries } = props; const radius = Math.min(width, height) / 2; @@ -38,7 +38,7 @@ const Radar: FC = props => { // 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); + adjustEntries(entries, quadrants, rings, radius, activeEntry); return ( diff --git a/plugins/tech-radar/src/components/Radar/utils.ts b/plugins/tech-radar/src/components/Radar/utils.ts index a8896ddf31..65268613fe 100644 --- a/plugins/tech-radar/src/components/Radar/utils.ts +++ b/plugins/tech-radar/src/components/Radar/utils.ts @@ -98,10 +98,10 @@ export const adjustQuadrants = ( export const adjustEntries = ( entries: Entry[], - activeEntry: Entry | null | undefined, quadrants: Quadrant[], rings: Ring[], radius: number, + activeEntry?: Entry | null, ) => { let seed = 42; entries.forEach((entry, idx) => { diff --git a/plugins/tech-radar/src/components/RadarBubble/RadarBubble.tsx b/plugins/tech-radar/src/components/RadarBubble/RadarBubble.tsx index dcf9a37ed7..723a408885 100644 --- a/plugins/tech-radar/src/components/RadarBubble/RadarBubble.tsx +++ b/plugins/tech-radar/src/components/RadarBubble/RadarBubble.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import React, { FC, useRef, useLayoutEffect } from 'react'; +import React, { useRef, useLayoutEffect } from 'react'; import { makeStyles, Theme } from '@material-ui/core'; type Props = { @@ -46,7 +46,7 @@ const useStyles = makeStyles(() => ({ }, })); -const RadarBubble: FC = props => { +const RadarBubble = (props: Props): JSX.Element => { const classes = useStyles(props); const { visible, text } = props; diff --git a/plugins/tech-radar/src/components/RadarComponent.tsx b/plugins/tech-radar/src/components/RadarComponent.tsx index 981d1f1369..8d56c6aa0f 100644 --- a/plugins/tech-radar/src/components/RadarComponent.tsx +++ b/plugins/tech-radar/src/components/RadarComponent.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import React, { useEffect, useState, FC } from 'react'; +import React, { useEffect, useState } from 'react'; import { Progress, useApi, errorApiRef, ErrorApi } from '@backstage/core'; import Radar from '../components/Radar'; import { TechRadarComponentProps, TechRadarLoaderResponse } from '../api'; @@ -22,15 +22,9 @@ import getSampleData from '../sampleData'; const useTechRadarLoader = (props: TechRadarComponentProps) => { const errorApi = useApi(errorApiRef); - const [state, setState] = useState<{ - loading: boolean; - error?: Error; - data?: TechRadarLoaderResponse; - }>({ - loading: true, - error: undefined, - data: undefined, - }); + const [error, setError] = useState(); + const [loading, setLoading] = useState(true); + const [data, setData] = useState(); const { getData } = props; @@ -41,22 +35,20 @@ const useTechRadarLoader = (props: TechRadarComponentProps) => { getData() .then((payload: TechRadarLoaderResponse) => { - setState({ loading: false, error: undefined, data: payload }); + setLoading(false); + setData(payload); }) .catch((err: Error) => { errorApi.post(err); - setState({ - loading: false, - error: err, - data: undefined, - }); + setLoading(false); + setError(err); }); }, [getData, errorApi]); - return state; + return { data, loading, error }; }; -const RadarComponent: FC = props => { +const RadarComponent = (props: TechRadarComponentProps): JSX.Element => { const { loading, error, data } = useTechRadarLoader(props); return ( diff --git a/plugins/tech-radar/src/components/RadarEntry/RadarEntry.tsx b/plugins/tech-radar/src/components/RadarEntry/RadarEntry.tsx index 6006979e67..ff499f4575 100644 --- a/plugins/tech-radar/src/components/RadarEntry/RadarEntry.tsx +++ b/plugins/tech-radar/src/components/RadarEntry/RadarEntry.tsx @@ -14,13 +14,14 @@ * limitations under the License. */ -import React, { FC } from 'react'; +import React from 'react'; import { makeStyles, Theme } from '@material-ui/core'; +import { WithLink } from '../../utils/components'; type Props = { x: number; y: number; - number: number; + value: number; color: string; url?: string; moved?: number; @@ -43,14 +44,27 @@ const useStyles = makeStyles(() => ({ }, })); -const RadarEntry: FC = props => { +const makeBlip = (color: string, moved?: number) => { + const style = { fill: color }; + + let blip = ; + if (moved && moved > 0) { + blip = ; // triangle pointing up + } else if (moved && moved < 0) { + blip = ; // triangle pointing down + } + + return blip; +}; + +const RadarEntry = (props: Props): JSX.Element => { const classes = useStyles(props); const { moved, color, url, - number, + value, x, y, onMouseEnter, @@ -58,24 +72,7 @@ const RadarEntry: FC = props => { onClick, } = props; - const style = { fill: color }; - - let blip; - if (moved && moved > 0) { - blip = ; // triangle pointing up - } else if (moved && moved < 0) { - blip = ; // triangle pointing down - } else { - blip = ; - } - - if (url) { - blip = ( - - {blip} - - ); - } + const blip = makeBlip(color, moved); return ( = props => { onMouseLeave={onMouseLeave} onClick={onClick} > - {blip} + + {blip} + - {number} + {value} ); diff --git a/plugins/tech-radar/src/components/RadarFooter/RadarFooter.tsx b/plugins/tech-radar/src/components/RadarFooter/RadarFooter.tsx index 7d60a63f59..b6284644eb 100644 --- a/plugins/tech-radar/src/components/RadarFooter/RadarFooter.tsx +++ b/plugins/tech-radar/src/components/RadarFooter/RadarFooter.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import React, { FC } from 'react'; +import React from 'react'; import { makeStyles, Theme } from '@material-ui/core'; type Props = { @@ -31,7 +31,7 @@ const useStyles = makeStyles(() => ({ }, })); -const RadarFooter: FC = props => { +const RadarFooter = (props: Props): JSX.Element => { const { x, y } = props; const classes = useStyles(props); diff --git a/plugins/tech-radar/src/components/RadarLegend/RadarLegend.tsx b/plugins/tech-radar/src/components/RadarLegend/RadarLegend.tsx index 03146a8071..c2509e9cac 100644 --- a/plugins/tech-radar/src/components/RadarLegend/RadarLegend.tsx +++ b/plugins/tech-radar/src/components/RadarLegend/RadarLegend.tsx @@ -13,9 +13,10 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import React, { FC } from 'react'; +import React from 'react'; import { makeStyles, Theme } from '@material-ui/core'; import { Quadrant, Ring, Entry } from '../../utils/types'; +import { WithLink } from '../../utils/components'; type Segments = { [k: number]: { [k: number]: Entry[] }; @@ -29,20 +30,7 @@ type Props = { onEntryMouseLeave?: (entry: Entry) => void; }; -const useStyles = makeStyles(() => ({ - quadrant: { - height: '100%', - width: '100%', - overflow: 'hidden', - pointerEvents: 'none', - }, - quadrantHeading: { - pointerEvents: 'none', - userSelect: 'none', - marginTop: 0, - marginBottom: 'calc(18px * 0.375)', - fontSize: '18px', - }, +const ringStyles = { rings: { columns: 3, }, @@ -69,6 +57,25 @@ const useStyles = makeStyles(() => ({ '-webkit-font-feature-settings': 'pnum', 'font-feature-settings': 'pnum', }, +}; + +const quadrantStyles = { + quadrant: { + height: '100%', + width: '100%', + overflow: 'hidden', + pointerEvents: 'none', + }, + quadrantHeading: { + pointerEvents: 'none', + userSelect: 'none', + marginTop: 0, + marginBottom: 'calc(18px * 0.375)', + fontSize: '18px', + }, +}; + +const entryStyle = { entry: { pointerEvents: 'none', userSelect: 'none', @@ -77,12 +84,18 @@ const useStyles = makeStyles(() => ({ entryLink: { pointerEvents: 'none', }, +}; + +const useStyles = makeStyles(() => ({ + ringStyles, + quadrantStyles, + entryStyle, })); -const RadarLegend: FC = props => { +const RadarLegend = (props: Props): JSX.Element => { const classes = useStyles(props); - const _getSegment = ( + const getSegment = ( segmented: Segments, quadrant: Quadrant, ring: Ring, @@ -94,7 +107,7 @@ const RadarLegend: FC = props => { return ridx === undefined ? [] : segmentedData[ridx + ringOffset] || []; }; - const _renderRing = ( + const renderRing = ( ring: Ring, entries: Entry[], onEntryMouseEnter?: Props['onEntryMouseEnter'], @@ -107,39 +120,29 @@ const RadarLegend: FC = props => {

(empty)

) : (
    - {entries.map(entry => { - let node = {entry.title}; - - if (entry.url) { - node = ( - - {node} - - ); - } - - return ( -
  1. onEntryMouseEnter(entry)) - } - onMouseLeave={ - onEntryMouseLeave && (() => onEntryMouseLeave(entry)) - } - > - {node} -
    2. - ); - })} + {entries.map(entry => ( +
  2. onEntryMouseEnter(entry)) + } + onMouseLeave={ + onEntryMouseLeave && (() => onEntryMouseLeave(entry)) + } + > + + {entry.title} + +
    3. + ))}
)} ); }; - const _renderQuadrant = ( + const renderQuadrant = ( segments: Segments, quadrant: Quadrant, rings: Ring[], @@ -158,9 +161,9 @@ const RadarLegend: FC = props => {

{quadrant.name}

{rings.map(ring => - _renderRing( + renderRing( ring, - _getSegment(segments, quadrant, ring), + getSegment(segments, quadrant, ring), onEntryMouseEnter, onEntryMouseLeave, ), @@ -171,7 +174,7 @@ const RadarLegend: FC = props => { ); }; - const _setupSegments = (entries: Entry[]) => { + const setupSegments = (entries: Entry[]) => { const segments: Segments = {}; for (const entry of entries) { @@ -209,12 +212,12 @@ const RadarLegend: FC = props => { onEntryMouseLeave, } = props; - const segments: Segments = _setupSegments(entries); + const segments: Segments = setupSegments(entries); return ( {quadrants.map(quadrant => - _renderQuadrant( + renderQuadrant( segments, quadrant, rings, diff --git a/plugins/tech-radar/src/components/RadarPage.tsx b/plugins/tech-radar/src/components/RadarPage.tsx index 4c7236e01f..1aefb72eca 100644 --- a/plugins/tech-radar/src/components/RadarPage.tsx +++ b/plugins/tech-radar/src/components/RadarPage.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import React, { FC } from 'react'; +import React from 'react'; import { Grid } from '@material-ui/core'; import { Content, @@ -29,7 +29,7 @@ import { import RadarComponent from '../components/RadarComponent'; import { techRadarApiRef, TechRadarApi } from '../api'; -const RadarPage: FC<{}> = () => { +const RadarPage = (): JSX.Element => { const techRadarApi = useApi(techRadarApiRef); return ( diff --git a/plugins/tech-radar/src/components/RadarPlot/RadarPlot.tsx b/plugins/tech-radar/src/components/RadarPlot/RadarPlot.tsx index 48ad660dde..0315afde5f 100644 --- a/plugins/tech-radar/src/components/RadarPlot/RadarPlot.tsx +++ b/plugins/tech-radar/src/components/RadarPlot/RadarPlot.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import React, { FC } from 'react'; +import React from 'react'; import { Quadrant, Ring, Entry } from '../../utils/types'; import RadarGrid from '../RadarGrid'; @@ -36,7 +36,7 @@ type Props = { }; // A component that draws the radar circle. -const RadarPlot: FC = props => { +const RadarPlot = (props: Props): JSX.Element => { const { width, height, @@ -71,7 +71,7 @@ const RadarPlot: FC = props => { x={entry.x || 0} y={entry.y || 0} color={entry.color || ''} - number={((entry && entry.idx) || 0) + 1} + value={((entry && entry.idx) || 0) + 1} url={entry.url} moved={entry.moved} onMouseEnter={onEntryMouseEnter && (() => onEntryMouseEnter(entry))} diff --git a/plugins/tech-radar/src/utils/components.tsx b/plugins/tech-radar/src/utils/components.tsx new file mode 100644 index 0000000000..af5a7b05bd --- /dev/null +++ b/plugins/tech-radar/src/utils/components.tsx @@ -0,0 +1,35 @@ +/* + * 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 from 'react'; + +type WithLinkProps = { + url?: string; + className: string; + children: React.ReactNode; +}; + +export const WithLink = ({ + url, + className, + children, +}: WithLinkProps): JSX.Element => + url ? ( + + {children} + + ) : ( + <>{children} + );