Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions src/Icon/Icon.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,8 @@ describe('<Icon />', () => {

{/* @ts-expect-error Using a non-existent icon from @openedx/paragon/icons is a type error */}
<Icon src={ParagonIcons.FooBarIcon} />
{/* @ts-expect-error The 'src' prop cannot be a string. */}
<Icon src="string" />
{/* @ts-expect-error Random props cannot be added */}
<Icon foo="bar" />
{/* @ts-expect-error This is not a valid size property */}
Comment thread
holaontiveros marked this conversation as resolved.
<Icon size="big" />
</CompileCheck>;
});
Expand Down
20 changes: 0 additions & 20 deletions src/Icon/index.d.ts

This file was deleted.

98 changes: 45 additions & 53 deletions src/Icon/index.jsx → src/Icon/index.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import React from 'react';
import PropTypes from 'prop-types';
import classNames from 'classnames';

import newId from '../utils/newId';
Expand All @@ -12,16 +11,53 @@ import withDeprecatedProps, { DeprTypes } from '../withDeprecatedProps';
* - focusable is set to false on the svg in all cases as a workaround for an ie11 bug
*/

interface SvgAttrs extends React.SVGAttributes<SVGElement> {
'aria-label'?: string;
'aria-labelledby'?: string;
'aria-hidden'?: boolean;
}

export interface IconProps extends Omit<React.ComponentPropsWithoutRef<'span'>, 'id' | 'className'> {
/**
* An icon component to render.
* Example import of a Paragon icon component: `import { Check } from '@openedx/paragon/icons';`
*/
src?: React.ComponentType<React.SVGAttributes<SVGElement>>;
/** HTML element attributes to pass through to the underlying svg element */
svgAttrs?: SvgAttrs;
/**
* the `id` property of the Icon element, by default this value is generated
* with the `newId` function with the `prefix` of `Icon`.
*/
id?: string | null;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where did null come from? It seems like id just accepted string | undefined before, so can we leave it at that?

/** The size of the icon. */
size?: 'xs' | 'sm' | 'md' | 'lg' | 'inline';
/** A class name that will define what the Icon looks like. */
className?: string | string[];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think our Paragon components generally just accept className?: string, so I don't think we should be adding new string[] functionality here (nor the corresponding Array.isArray(className) ? className.join(' ') : code below).

Fine to do that in a separate PR if you'd like, but I think it's better for className to behave consistently across all Paragon components.

/**
* a boolean that determines the value of `aria-hidden` attribute on the Icon span,
* this value is `true` by default.
*/
hidden?: boolean;
/**
* a string or an element that will be used on a secondary span leveraging the `sr-only` style
* for screenreader only text, this value is `undefined` by default. This value is recommended for use unless
* the Icon is being used in a way that is purely decorative or provides no additional context for screen
* reader users. This field should be thought of the same way an `alt` attribute would be used for `image` tags.
*/
screenReaderText?: React.ReactNode;
}

function Icon({
src: Component,
id,
className,
hidden,
hidden = true,
screenReaderText,
svgAttrs,
svgAttrs = {},
size,
...attrs
}) {
}: IconProps) {
if (Component) {
// If no aria label is specified, hide this icon from screenreaders
const hasAriaLabel = svgAttrs['aria-label'] || svgAttrs['aria-labelledby'];
Expand All @@ -35,8 +71,8 @@ function Icon({

return (
<span
className={classNames('pgn__icon', { [`pgn__icon__${size}`]: !!size }, className)}
id={id}
className={classNames('pgn__icon', { [`pgn__icon__${size}`]: !!size }, Array.isArray(className) ? className.join(' ') : className)}
id={id || undefined}
{...attrs}
>
<Component
Expand All @@ -57,7 +93,7 @@ function Icon({
<>
<span
id={id || newId('Icon')}
className={className}
className={Array.isArray(className) ? className.join(' ') : className}
aria-hidden={hidden}
/>
{screenReaderText && (
Expand All @@ -69,55 +105,11 @@ function Icon({
);
}

Icon.propTypes = {
/**
* An icon component to render.
* Example import of a Paragon icon component: `import { Check } from '@openedx/paragon/icons';`
*/
src: PropTypes.elementType,
/** HTML element attributes to pass through to the underlying svg element */
svgAttrs: PropTypes.shape({
'aria-label': PropTypes.string,
'aria-labelledby': PropTypes.string,
}),
/**
* the `id` property of the Icon element, by default this value is generated
* with the `newId` function with the `prefix` of `Icon`.
*/
id: PropTypes.string,
/** The size of the icon. */
size: PropTypes.oneOf(['xs', 'sm', 'md', 'lg']),
/** A class name that will define what the Icon looks like. */
className: PropTypes.string,
/**
* a boolean that determines the value of `aria-hidden` attribute on the Icon span,
* this value is `true` by default.
*/
hidden: PropTypes.bool,
/**
* a string or an element that will be used on a secondary span leveraging the `sr-only` style
* for screenreader only text, this value is `undefined` by default. This value is recommended for use unless
* the Icon is being used in a way that is purely decorative or provides no additional context for screen
* reader users. This field should be thought of the same way an `alt` attribute would be used for `image` tags.
*/
screenReaderText: PropTypes.oneOfType([PropTypes.string, PropTypes.element]),
};

Icon.defaultProps = {
src: null,
svgAttrs: {},
id: undefined,
hidden: true,
screenReaderText: undefined,
size: undefined,
className: undefined,
};

export default withDeprecatedProps(Icon, 'Icon', {
className: {
deprType: DeprTypes.FORMAT,
expect: value => typeof value === 'string',
transform: value => (Array.isArray(value) ? value.join(' ') : value),
expect: (value: any) => typeof value === 'string',
transform: (value: any) => (Array.isArray(value) ? value.join(' ') : value),
message: 'It should be a string.',
},
});