Skip to content

Commit

Permalink
[Enhance] Implement Validation for Icon Input Source & Set Default Ic…
Browse files Browse the repository at this point in the history
…on to `Beaker` (#1137)

* Revise the logic to handle default icon

Signed-off-by: Willie Hung <[email protected]>

* Categorize different input icon types

Signed-off-by: Willie Hung <[email protected]>

* Remove unused code

Signed-off-by: Willie Hung <[email protected]>

* Update CHANGELOG.md

Signed-off-by: Willie Hung <[email protected]>

* Use readable parameter name

Signed-off-by: Willie Hung <[email protected]>

* Handle different input cases and add tests

Signed-off-by: Willie Hung <[email protected]>

---------

Signed-off-by: Willie Hung <[email protected]>
Co-authored-by: Josh Romero <[email protected]>
  • Loading branch information
willie-hung and joshuarrrr authored Dec 8, 2023
1 parent 24592a6 commit 0d3b6cc
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 38 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
- Add slugs to markdown headers ([#1051](https://github.com/opensearch-project/oui/pull/1051))
- Add `crossInCircleEmpty` and `power` icons ([#1044](https://github.com/opensearch-project/oui/pull/1044))
- Add `Figma` icon and link to OUI Figma resources ([#1064](https://github.com/opensearch-project/oui/pull/1064))
- Implement validation for icon input source & set default icon to `Beaker` ([#1137](https://github.com/opensearch-project/oui/pull/1137))

### 🐛 Bug Fixes

Expand Down
52 changes: 51 additions & 1 deletion src/components/icon/__snapshots__/icon.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -10184,7 +10184,7 @@ exports[`OuiIcon props type wrench is rendered 1`] = `
</svg>
`;

exports[`OuiIcon renders custom components 1`] = `
exports[`OuiIcon render different types of icons renders custom components 1`] = `
<span
aria-hidden="true"
aria-label="heart"
Expand All @@ -10195,3 +10195,53 @@ exports[`OuiIcon renders custom components 1`] = `
❤️
</span>
`;

exports[`OuiIcon render different types of icons renders custom svg from absolute url 1`] = `
<img
alt=""
class="ouiIcon ouiIcon--medium"
src="https://upload.wikimedia.org/wikipedia/commons/0/02/SVG_logo.svg"
/>
`;

exports[`OuiIcon render different types of icons renders custom svg from relative url 1`] = `
<img
alt=""
class="ouiIcon ouiIcon--medium"
src="./assets/beaker.svg"
/>
`;

exports[`OuiIcon render different types of icons renders default icon when type is not in OuiIconType 1`] = `
<svg
aria-hidden="true"
class="ouiIcon ouiIcon--medium ouiIcon-isLoading"
focusable="false"
height="16"
role="img"
viewBox="0 0 16 16"
width="16"
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M5.277 10.088c.02.014.04.03.057.047.582.55 1.134.812 1.666.812.586 0 1.84-.293 3.713-.88L9 6.212V2H7v4.212l-1.723 3.876Zm-.438.987L3.539 14h8.922l-1.32-2.969C9.096 11.677 7.733 12 7 12c-.74 0-1.463-.315-2.161-.925ZM6 2H5V1h6v1h-1v4l3.375 7.594A1 1 0 0 1 12.461 15H3.54a1 1 0 0 1-.914-1.406L6 6V2Z"
/>
</svg>
`;

exports[`OuiIcon render different types of icons renders icon when type is in cache 1`] = `
<svg
aria-hidden="true"
class="ouiIcon ouiIcon--medium"
focusable="false"
height="16"
role="img"
viewBox="0 0 16 16"
width="16"
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M0 1.994C0 .893.895 0 1.994 0h12.012C15.107 0 16 .895 16 1.994v12.012A1.995 1.995 0 0 1 14.006 16H1.994A1.995 1.995 0 0 1 0 14.006V1.994Zm1 0v12.012c0 .548.446.994.994.994h12.012a.995.995 0 0 0 .994-.994V1.994A.995.995 0 0 0 14.006 1H1.994A.995.995 0 0 0 1 1.994ZM1 4h14v1H1V4Zm1.5-1a.5.5 0 0 1 0-1h1a.5.5 0 0 1 0 1h-1Zm3 0a.5.5 0 0 1 0-1h1a.5.5 0 0 1 0 1h-1Zm4.947 6.106a1 1 0 0 1 0 1.788l-3 2A1 1 0 0 1 6 12V8a1 1 0 0 1 1.447-.894l3 2ZM10 10 7 8v4l3-2Z"
/>
</svg>
`;
55 changes: 41 additions & 14 deletions src/components/icon/icon.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -135,23 +135,11 @@ describe('OuiIcon', () => {
});
});

it('renders custom components', () => {
const CustomIcon = ({ ...props }) => {
return (
<span role="img" aria-label="heart" {...props}>
❤️
</span>
);
};
const component = mount(<OuiIcon type={CustomIcon} />);
expect(prettyHtml(component.html())).toMatchSnapshot();
});

describe('appendIconComponentCache', () => {
it('does nothing if not called', () => {
const component = mount(<OuiIcon type="videoPlayer" />);
expect(
component.find('OuiIcon[type="videoPlayer"] > OuiIconEmpty').length
component.find('OuiIcon[type="videoPlayer"] > OuiIconBeaker').length
).toBe(1);
});

Expand All @@ -172,8 +160,47 @@ describe('OuiIcon', () => {
});
const component = mount(<OuiIcon type="accessibility" />);
expect(
component.find('OuiIcon[type="accessibility"] > OuiIconEmpty').length
component.find('OuiIcon[type="accessibility"] > OuiIconBeaker').length
).toBe(1);
});
});

describe('render different types of icons', () => {
it('renders icon when type is in cache', () => {
appendIconComponentCache({
videoPlayer: OuiIconVideoPlayer,
});
const component = mount(<OuiIcon type="videoPlayer" />);
expect(prettyHtml(component.html())).toMatchSnapshot();
});

it('renders custom svg from absolute url', () => {
const component = mount(
<OuiIcon type="https://upload.wikimedia.org/wikipedia/commons/0/02/SVG_logo.svg" />
);
expect(prettyHtml(component.html())).toMatchSnapshot();
});

it('renders custom svg from relative url', () => {
const component = mount(<OuiIcon type="./assets/beaker.svg" />);
expect(prettyHtml(component.html())).toMatchSnapshot();
});

it('renders default icon when type is not in OuiIconType', () => {
const component = mount(<OuiIcon type="foo" />);
expect(prettyHtml(component.html())).toMatchSnapshot();
});

it('renders custom components', () => {
const CustomIcon = ({ ...props }) => {
return (
<span role="img" aria-label="heart" {...props}>
❤️
</span>
);
};
const component = mount(<OuiIcon type={CustomIcon} />);
expect(prettyHtml(component.html())).toMatchSnapshot();
});
});
});
52 changes: 29 additions & 23 deletions src/components/icon/icon.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ import { CommonProps, keysOf } from '../common';
// because we'd need to dynamically know if we're importing the
// TS file (dev/docs) or the JS file (distributed), and it's more effort than worth
// to generate & git track a TS module definition for each icon component
import { icon as empty } from './assets/empty.js';
import { icon as defaultIcon } from './assets/beaker.js';
import { enqueueStateChange } from '../../services/react';

import { htmlIdGenerator } from '../../services';
Expand Down Expand Up @@ -582,22 +582,16 @@ interface State {
neededLoading: boolean; // controls the fade-in animation, cached icons are immediately rendered
}

function isOuiIconType(x: OuiIconProps['type']): x is OuiIconType {
return typeof x === 'string' && typeToPathMap.hasOwnProperty(x);
function isOuiIconType(type: OuiIconProps['type']): type is OuiIconType {
return typeof type === 'string' && typeToPathMap.hasOwnProperty(type);
}

function getInitialIcon(icon: OuiIconProps['type']) {
if (icon == null) {
return undefined;
}
if (isOuiIconType(icon)) {
if (iconComponentCache.hasOwnProperty(icon)) {
return iconComponentCache[icon];
}
return undefined;
}
function isUrl(type: OuiIconProps['type']) {
return typeof type === 'string' && (type.includes('.') || type.includes('/'));
}

return icon;
function isCachedIcon(type: OuiIconProps['type']) {
return isOuiIconType(type) && iconComponentCache.hasOwnProperty(type);
}

const generateId = htmlIdGenerator();
Expand Down Expand Up @@ -628,13 +622,22 @@ export class OuiIcon extends PureComponent<OuiIconProps, State> {
super(props);

const { type } = props;
const initialIcon = getInitialIcon(type);
let initialIcon;
let isLoading = false;

if (isOuiIconType(type) && initialIcon == null) {
// Category 1: cached oui icons
if (isCachedIcon(type)) {
initialIcon = iconComponentCache[type as string];
// Category 2: URL (relative, absolute)
} else if (isUrl(type)) {
initialIcon = type;
// Category 3: non-cached oui icon or new icon
} else if (typeof type === 'string') {
isLoading = true;
this.loadIconComponent(type);
this.loadIconComponent(type as OuiIconType);
} else {
// Category 4: custom icon component
initialIcon = type;
this.onIconLoad();
}

Expand Down Expand Up @@ -683,12 +686,14 @@ export class OuiIcon extends PureComponent<OuiIconProps, State> {
return;
}

const iconPath = typeToPathMap[iconType] || 'beaker';

import(
/* webpackChunkName: "icon.[request]" */
// It's important that we don't use a template string here, it
// stops webpack from building a dynamic require context.
// eslint-disable-next-line prefer-template
'./assets/' + typeToPathMap[iconType] + '.js'
'./assets/' + iconPath + '.js'
).then(({ icon }) => {
iconComponentCache[iconType] = icon;
enqueueStateChange(() => {
Expand Down Expand Up @@ -761,7 +766,7 @@ export class OuiIcon extends PureComponent<OuiIconProps, State> {
className
);

const icon = this.state.icon || (empty as ComponentType);
const icon = this.state.icon || (defaultIcon as ComponentType);

// This is a fix for IE and Edge, which ignores tabindex="-1" on an SVG, but respects
// focusable="false".
Expand All @@ -771,6 +776,7 @@ export class OuiIcon extends PureComponent<OuiIconProps, State> {
// - For all other values, the consumer wants the icon to be focusable.
const focusable = tabIndex == null || tabIndex === -1 ? 'false' : 'true';

// relative, absolute path
if (typeof icon === 'string') {
return (
<img
Expand All @@ -784,15 +790,15 @@ export class OuiIcon extends PureComponent<OuiIconProps, State> {
} else {
const Svg = icon;

// If it's an empty icon, or if there is no aria-label, aria-labelledby, or title it gets aria-hidden true
// If it's a default icon, or if there is no aria-label, aria-labelledby, or title it gets aria-hidden true
const isAriaHidden =
icon === empty ||
icon === defaultIcon ||
!(
this.props['aria-label'] ||
this.props['aria-labelledby'] ||
this.props.title
);
const hideIconEmpty = isAriaHidden && { 'aria-hidden': true };
const hideDefaultIcon = isAriaHidden && { 'aria-hidden': true };

let titleId: any;

Expand All @@ -817,7 +823,7 @@ export class OuiIcon extends PureComponent<OuiIconProps, State> {
title={title}
{...titleId}
{...rest}
{...hideIconEmpty}
{...hideDefaultIcon}
/>
);
}
Expand Down

0 comments on commit 0d3b6cc

Please sign in to comment.