Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

Commit

Permalink
fix(Icon): make "name" prop required (#1723)
Browse files Browse the repository at this point in the history
* make Icon "name" prop required

* update changelog

* Update CHANGELOG.md

Co-Authored-By: Oleksandr Fediashov <[email protected]>

* update propTypes

* fix failing tests

* Fix changelog change
  • Loading branch information
lucivpav authored Jul 30, 2019
1 parent aa1b907 commit b70bee7
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 8 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Expose `isFromKeyboard` in `Grid` component @sophieH29 ([#1729](https://github.com/stardust-ui/react/pull/1729))
- Add `onActiveIndexChange` prop to `Tree` component @lucivpav ([#1728](https://github.com/stardust-ui/react/pull/1728))

### Fixes
- Require `name` prop in `Icon` component @lucivpav ([#1723](https://github.com/stardust-ui/react/pull/1723))

### Documentation
- Fix code in changing component variables section of theming examples @lucivpav ([#1626](https://github.com/stardust-ui/react/pull/1626))

Expand Down
4 changes: 2 additions & 2 deletions packages/react/src/components/Icon/Icon.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export interface IconProps extends UIComponentProps, ColorComponentProps {
disabled?: boolean

/** Name of the icon. */
name?: string
name: string

/** An icon can provide an outline variant. */
outline?: boolean
Expand Down Expand Up @@ -61,7 +61,7 @@ class Icon extends UIComponent<WithAsProp<IconProps>, any> {
bordered: PropTypes.bool,
circular: PropTypes.bool,
disabled: PropTypes.bool,
name: PropTypes.string,
name: PropTypes.string.isRequired,
outline: PropTypes.bool,
rotate: PropTypes.number,
size: customPropTypes.size,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { Props, PropsOf, InstanceOf } from 'src/types'

export type ShorthandTestOptions<TProps = any> = {
mapsValueToProp: keyof (TProps & React.HTMLProps<HTMLElement>) | false
requiredProps?: any
}

export const DefaultShorthandTestOptions: ShorthandTestOptions = {
Expand Down Expand Up @@ -65,7 +66,11 @@ export default ((Component: React.ComponentType) => {
}

test(`object value is spread as ${displayName}'s props`, () => {
expectShorthandPropsAreHandled({ foo: 'foo value', bar: 'bar value' })
expectShorthandPropsAreHandled({
...options.requiredProps,
foo: 'foo value',
bar: 'bar value',
})
})
})
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@ describe('Attachment', () => {
isConformant(Attachment)
attachmentImplementsShorthandProp('header', Text)
attachmentImplementsShorthandProp('description', Text)
attachmentImplementsShorthandProp('icon', Icon, { mapsValueToProp: 'name' })
attachmentImplementsShorthandProp('icon', Icon, {
mapsValueToProp: 'name',
requiredProps: { name: 'at' },
})
attachmentImplementsShorthandProp('action', Button)

describe('accessibility', () => {
Expand Down
5 changes: 4 additions & 1 deletion packages/react/test/specs/components/Button/Button-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ const buttonImplementsShorthandProp = implementsShorthandProp(Button)

describe('Button', () => {
isConformant(Button)
buttonImplementsShorthandProp('icon', Icon, { mapsValueToProp: 'name' })
buttonImplementsShorthandProp('icon', Icon, {
mapsValueToProp: 'name',
requiredProps: { name: 'at' },
})

describe('accessibility', () => {
describe('button', () => {
Expand Down
5 changes: 4 additions & 1 deletion packages/react/test/specs/components/Icon/Icon-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,14 @@ import { mountWithProviderAndGetComponent } from 'test/utils'
import { ThemeInput } from 'src/themes/types'

describe('Icon', () => {
isConformant(Icon)
isConformant(Icon, { requiredProps: { name: 'at' } })

describe('accessibility', () => {
handlesAccessibility(Icon, {
defaultRootRole: 'img',
requiredProps: {
name: 'at',
},
})

describe('aria-hidden', () => {
Expand Down
5 changes: 4 additions & 1 deletion packages/react/test/specs/components/Input/Input-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,10 @@ describe('Input', () => {
})

implementsShorthandProp(Input)('input', Box, { mapsValueToProp: 'type' })
implementsShorthandProp(Input)('icon', Icon, { mapsValueToProp: 'name' })
implementsShorthandProp(Input)('icon', Icon, {
mapsValueToProp: 'name',
requiredProps: { name: 'at' },
})

describe('wrapper', () => {
implementsShorthandProp(Input)('wrapper', Box, { mapsValueToProp: 'children' })
Expand Down
5 changes: 4 additions & 1 deletion packages/react/test/specs/components/Label/Label-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ const labelImplementsShorthandProp = implementsShorthandProp(Label)

describe('Label', () => {
isConformant(Label)
labelImplementsShorthandProp('icon', Icon, { mapsValueToProp: 'name' })
labelImplementsShorthandProp('icon', Icon, {
mapsValueToProp: 'name',
requiredProps: { name: 'at' },
})
labelImplementsShorthandProp('image', Image, { mapsValueToProp: 'src' })
})

0 comments on commit b70bee7

Please sign in to comment.