From 6f18fe661dab027b4819210224f7f8531639e115 Mon Sep 17 00:00:00 2001 From: Jose Cortinas Date: Tue, 13 Feb 2024 15:01:54 -0600 Subject: [PATCH] Update Octicon component for simpler structure --- app/src/ui/octicons/octicon.tsx | 63 +++++++++++++-------------------- app/test/unit/octicon-test.ts | 12 +++---- 2 files changed, 30 insertions(+), 45 deletions(-) diff --git a/app/src/ui/octicons/octicon.tsx b/app/src/ui/octicons/octicon.tsx index 09fe0a0075..790c11b54a 100644 --- a/app/src/ui/octicons/octicon.tsx +++ b/app/src/ui/octicons/octicon.tsx @@ -1,6 +1,5 @@ import * as React from 'react' -import { OcticonSymbolType, OcticonSymbolSize } from './octicons.generated' -import { CustomOcticonSymbolType } from '.' +import { OcticonSymbol, OcticonSymbolVariant } from '.' import classNames from 'classnames' import ReactDOM from 'react-dom' import { createObservableRef } from '../lib/observable-ref' @@ -12,7 +11,7 @@ interface IOcticonProps { * or CustomOcticonSymbolType type. Supports custom paths as well as * those provided through the static properties of the OcticonSymbol class. */ - readonly symbol: OcticonSymbolType | CustomOcticonSymbolType + readonly symbol: OcticonSymbol /** * An optional classname that will be appended to the default @@ -27,7 +26,7 @@ interface IOcticonProps { readonly tooltipDirection?: TooltipDirection - readonly height?: OcticonSymbolSize + readonly height?: number } /** @@ -37,7 +36,7 @@ interface IOcticonProps { * which is why the width and height properties specify the maximum and * not the minimum size. * - * Usage: `` + * Usage: `` */ export class Octicon extends React.Component { private svgRef = createObservableRef() @@ -45,43 +44,27 @@ export class Octicon extends React.Component { public render() { const { symbol } = this.props - if (this.isCustomSymbol(symbol)) { - return this.renderIcon( - symbol.s ?? 'custom', - symbol.h, - symbol.w, - Array.from(symbol.d) - ) + if (this.isSingleVariant(symbol)) { + return this.renderIcon(symbol.p, symbol.h, symbol.w) } else { const height = this.props.height ?? 16 const naturalHeight = this.closestNaturalHeight( - Object.keys(symbol.v).map(h => - parseInt(h, 10) - ) as Array, + Object.keys(symbol).map(h => parseInt(h, 10)) as Array, height ) - const scaledSymbol = symbol.v[naturalHeight]! + const scaledSymbol = symbol[naturalHeight]! const naturalWidth = scaledSymbol.w const width = height * (naturalWidth / naturalHeight) - return this.renderIcon(symbol.s, height, width, scaledSymbol.p) + return this.renderIcon(scaledSymbol.p, height, width) } } - private renderIcon( - name: string, - height: number, - width: number, - paths: string[] - ) { + private renderIcon(paths: string[], height: number, width: number) { const { title, tooltipDirection } = this.props const viewBox = `0 0 ${width} ${height}` - const className = classNames( - 'octicon', - `octicon-${name}`, - this.props.className - ) + const className = classNames('octicon', this.props.className) // Hide the octicon from screen readers when it's only being used // as a visual without any attached meaning applicable to users @@ -117,21 +100,23 @@ export class Octicon extends React.Component { ) } - private closestNaturalHeight( - naturalHeights: Array, - height: OcticonSymbolSize - ) { + private isSingleVariant( + symbol: OcticonSymbol + ): symbol is OcticonSymbolVariant { + return ( + symbol instanceof Object && + symbol.hasOwnProperty('p') && + symbol.hasOwnProperty('h') && + symbol.hasOwnProperty('w') + ) + } + + private closestNaturalHeight(naturalHeights: Array, height: number) { return naturalHeights.reduce( (acc, naturalHeight) => (naturalHeight <= height ? naturalHeight : acc), naturalHeights[0] ) } - - private isCustomSymbol( - symbol: OcticonSymbolType | CustomOcticonSymbolType - ): symbol is CustomOcticonSymbolType { - return (symbol as CustomOcticonSymbolType).d !== undefined - } } /** @@ -142,7 +127,7 @@ export class Octicon extends React.Component { * @param id Optional identifier to set to the wrapper element. */ export function createOcticonElement( - symbol: OcticonSymbolType, + symbol: OcticonSymbol, className?: string, id?: string ) { diff --git a/app/test/unit/octicon-test.ts b/app/test/unit/octicon-test.ts index 0fabeed50b..b87e6d895f 100644 --- a/app/test/unit/octicon-test.ts +++ b/app/test/unit/octicon-test.ts @@ -1,5 +1,5 @@ import { iconForRepository } from '../../src/ui/octicons' -import * as OcticonSymbol from '../../src/ui/octicons/octicons.generated' +import * as octicons from '../../src/ui/octicons/octicons.generated' import { CloningRepository } from '../../src/models/cloning-repository' import { Repository } from '../../src/models/repository' import { gitHubRepoFixture } from '../helpers/github-repo-builder' @@ -11,13 +11,13 @@ describe('octicon/iconForRepository', () => { 'https://github.com/desktop/desktop' ) const icon = iconForRepository(repository) - expect(icon).toEqual(OcticonSymbol.desktopDownload) + expect(icon).toEqual(octicons.desktopDownload) }) it('shows computer icon for non-GitHub repository', () => { const repository = new Repository('C:/some/path/to/repo', 1, null, false) const icon = iconForRepository(repository) - expect(icon).toEqual(OcticonSymbol.deviceDesktop) + expect(icon).toEqual(octicons.deviceDesktop) }) it('shows repo icon for public GitHub repository', () => { @@ -33,7 +33,7 @@ describe('octicon/iconForRepository', () => { false ) const icon = iconForRepository(repository) - expect(icon).toEqual(OcticonSymbol.repo) + expect(icon).toEqual(octicons.repo) }) it('shows lock icon for private GitHub repository', () => { @@ -49,7 +49,7 @@ describe('octicon/iconForRepository', () => { false ) const icon = iconForRepository(repository) - expect(icon).toEqual(OcticonSymbol.lock) + expect(icon).toEqual(octicons.lock) }) it('shows fork icon for forked GitHub repository', () => { @@ -66,6 +66,6 @@ describe('octicon/iconForRepository', () => { false ) const icon = iconForRepository(repository) - expect(icon).toEqual(OcticonSymbol.repoForked) + expect(icon).toEqual(octicons.repoForked) }) })