From 04834a47b895f9c2b70e1b62f9c9bb4af19b442e Mon Sep 17 00:00:00 2001 From: Oleksandr Fediashov Date: Wed, 19 Feb 2020 14:37:11 +0100 Subject: [PATCH 1/6] proto: use type safe "as" --- .../componentGenerators.ts | 2 +- docs/src/components/Logo/Logo.tsx | 2 +- .../components/Image/Types/ImageExample.tsx | 21 ++++++++++++++++- .../react/src/components/Avatar/Avatar.tsx | 2 +- .../src/components/Dropdown/DropdownItem.tsx | 2 +- .../Dropdown/DropdownSelectedItem.tsx | 2 +- packages/react/src/components/Image/Image.tsx | 23 ++++++++++--------- packages/react/src/components/Label/Label.tsx | 2 +- .../teams/components/Image/imageStyles.ts | 2 +- packages/react/src/types.ts | 6 +++++ yarn.lock | 6 ++--- 11 files changed, 48 insertions(+), 22 deletions(-) diff --git a/docs/src/components/ComponentPlayground/componentGenerators.ts b/docs/src/components/ComponentPlayground/componentGenerators.ts index 766aa87560..4565a4b780 100644 --- a/docs/src/components/ComponentPlayground/componentGenerators.ts +++ b/docs/src/components/ComponentPlayground/componentGenerators.ts @@ -71,7 +71,7 @@ export const Icon: KnobComponentGenerators = { }, } -export const Image: KnobComponentGenerators = { +export const Image: KnobComponentGenerators> = { src: ({ propName }) => ({ hook: useStringKnob, name: propName, diff --git a/docs/src/components/Logo/Logo.tsx b/docs/src/components/Logo/Logo.tsx index 0c0b026497..f7877fe8ac 100644 --- a/docs/src/components/Logo/Logo.tsx +++ b/docs/src/components/Logo/Logo.tsx @@ -1,7 +1,7 @@ import * as React from 'react' import { Image, ImageProps } from '@fluentui/react' -export type LogoProps = ImageProps & { +export type LogoProps = ImageProps & { flavor?: 'black' | 'white' | 'inverted' } diff --git a/docs/src/examples/components/Image/Types/ImageExample.tsx b/docs/src/examples/components/Image/Types/ImageExample.tsx index 231d49c840..fd33bfb394 100644 --- a/docs/src/examples/components/Image/Types/ImageExample.tsx +++ b/docs/src/examples/components/Image/Types/ImageExample.tsx @@ -1,6 +1,25 @@ import * as React from 'react' import { Image } from '@fluentui/react' -const ImageExample = () => +const ImageExample = () => ( + <> + {/* Wrong default props */} + {/* */} + {/* */} + + {/* Wrong elements */} + {/* */} + {/* */} + + {/* Wrong attrs on els */} + {/* */} + {/* */} + {/* */} + + {/* Wrong attr on components */} + {/* */} + {/* */} + +) export default ImageExample diff --git a/packages/react/src/components/Avatar/Avatar.tsx b/packages/react/src/components/Avatar/Avatar.tsx index bf50e2cc18..2a663b6dc2 100644 --- a/packages/react/src/components/Avatar/Avatar.tsx +++ b/packages/react/src/components/Avatar/Avatar.tsx @@ -31,7 +31,7 @@ export interface AvatarProps extends UIComponentProps { accessibility?: Accessibility /** Shorthand for the image. */ - image?: ShorthandValue + image?: ShorthandValue> /** Shorthand for the label. */ label?: ShorthandValue diff --git a/packages/react/src/components/Dropdown/DropdownItem.tsx b/packages/react/src/components/Dropdown/DropdownItem.tsx index 9c5d02a181..0424eca30d 100644 --- a/packages/react/src/components/Dropdown/DropdownItem.tsx +++ b/packages/react/src/components/Dropdown/DropdownItem.tsx @@ -44,7 +44,7 @@ export interface DropdownItemProps extends UIComponentProps { header?: ShorthandValue /** Item's image. */ - image?: ShorthandValue + image?: ShorthandValue> /** Indicated whether the item has been set active by keyboard. */ isFromKeyboard?: boolean diff --git a/packages/react/src/components/Dropdown/DropdownSelectedItem.tsx b/packages/react/src/components/Dropdown/DropdownSelectedItem.tsx index 1faac19891..b2c28dbf71 100644 --- a/packages/react/src/components/Dropdown/DropdownSelectedItem.tsx +++ b/packages/react/src/components/Dropdown/DropdownSelectedItem.tsx @@ -42,7 +42,7 @@ export interface DropdownSelectedItemProps extends UIComponentProps /** Image of the selected item. */ - image?: ShorthandValue + image?: ShorthandValue> /** * Called on selected item click. diff --git a/packages/react/src/components/Image/Image.tsx b/packages/react/src/components/Image/Image.tsx index 805bc7a780..92dfaa6ab6 100644 --- a/packages/react/src/components/Image/Image.tsx +++ b/packages/react/src/components/Image/Image.tsx @@ -17,14 +17,13 @@ import * as React from 'react' import { ThemeContext } from 'react-fela' import { createShorthandFactory, UIComponentProps, commonPropTypes } from '../../utils' -import { - FluentComponentStaticProps, - ProviderContextPrepared, - WithAsProp, - withSafeTypeForAs, -} from '../../types' +import { PropsOfElement, ProviderContextPrepared } from '../../types' + +export interface ImageOwnProps + extends UIComponentProps, + ImageBehaviorProps { + as?: E -export interface ImageProps extends UIComponentProps, ImageBehaviorProps { /** Alternative text. */ alt?: string @@ -46,7 +45,10 @@ export interface ImageProps extends UIComponentProps, ImageBehaviorProps { src?: string } -const Image: React.FC> & FluentComponentStaticProps = props => { +export type ImageProps = ImageOwnProps & + Omit, keyof ImageOwnProps> + +function Image(props: ImageProps): React.ReactElement { const context: ProviderContextPrepared = React.useContext(ThemeContext) const { setStart, setEnd } = useTelemetry(Image.displayName, context.telemetry) setStart() @@ -102,11 +104,11 @@ const Image: React.FC> & FluentComponentStaticProps(Image) +export default Image diff --git a/packages/react/src/components/Label/Label.tsx b/packages/react/src/components/Label/Label.tsx index 2e57a1f5af..2fc4de10cc 100644 --- a/packages/react/src/components/Label/Label.tsx +++ b/packages/react/src/components/Label/Label.tsx @@ -60,7 +60,7 @@ export interface LabelProps iconPosition?: 'start' | 'end' /** A Label can contain an image. */ - image?: ShorthandValue + image?: ShorthandValue> /** A Label can position its image at the start or end of the layout. */ imagePosition?: 'start' | 'end' diff --git a/packages/react/src/themes/teams/components/Image/imageStyles.ts b/packages/react/src/themes/teams/components/Image/imageStyles.ts index f23037f521..00946a890b 100644 --- a/packages/react/src/themes/teams/components/Image/imageStyles.ts +++ b/packages/react/src/themes/teams/components/Image/imageStyles.ts @@ -2,7 +2,7 @@ import { ComponentSlotStylesPrepared, ICSSInJSStyle } from '@fluentui/styles' import { ImageProps } from '../../../../components/Image/Image' import { ImageVariables } from './imageVariables' -export type ImageStylesProps = Pick +export type ImageStylesProps = Pick, 'avatar' | 'circular' | 'fluid'> const imageStyles: ComponentSlotStylesPrepared = { root: ({ props, variables }): ICSSInJSStyle => ({ diff --git a/packages/react/src/types.ts b/packages/react/src/types.ts index c979941e51..7a4aa8b096 100644 --- a/packages/react/src/types.ts +++ b/packages/react/src/types.ts @@ -19,6 +19,12 @@ export type DebounceResultFn = T & { // Utilities // ======================================================== +// Source: https://github.com/emotion-js/emotion/blob/master/packages/styled-base/types/helper.d.ts +export type PropsOfElement< + // eslint-disable-next-line @typescript-eslint/no-explicit-any + E extends keyof JSX.IntrinsicElements | React.JSXElementConstructor +> = JSX.LibraryManagedAttributes> + export type ResultOf = T extends (...arg: any[]) => infer TResult ? TResult : never export type ObjectOf = { [key: string]: T } diff --git a/yarn.lock b/yarn.lock index a2ec702472..9e5dea8917 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5203,9 +5203,9 @@ "@types/react" "*" "@types/react@*", "@types/react@^16.8.10": - version "16.8.10" - resolved "https://registry.yarnpkg.com/@types/react/-/react-16.8.10.tgz#1ccb6fde17f71a62ef055382ec68bdc379d4d8d9" - integrity sha512-7bUQeZKP4XZH/aB4i7k1i5yuwymDu/hnLMhD9NjVZvQQH7ZUgRN3d6iu8YXzx4sN/tNr0bj8jgguk8hhObzGvA== + version "16.9.20" + resolved "https://registry.yarnpkg.com/@types/react/-/react-16.9.20.tgz#e83285766340fb1a7fafe7e5c4708c53832e3641" + integrity sha512-jRrWBr25zzEVNa4QbESKLPluvrZ3W6Odfwrfe2F5vzbrDuNvlpnHa/xbZcXg8RH5D4CE181J5VxrRrLvzRH+5A== dependencies: "@types/prop-types" "*" csstype "^2.2.0" From f5cf147c724be87dc11e72a308616e6cd4f3182f Mon Sep 17 00:00:00 2001 From: Oleksandr Fediashov Date: Wed, 19 Feb 2020 17:17:26 +0100 Subject: [PATCH 2/6] wip --- .../componentGenerators.ts | 2 +- docs/src/components/Logo/Logo.tsx | 2 +- .../components/Image/Types/ImageExample.tsx | 21 +------------------ .../src/components/Dropdown/DropdownItem.tsx | 2 +- .../Dropdown/DropdownSelectedItem.tsx | 2 +- packages/react/src/components/Image/Image.tsx | 2 +- packages/react/src/components/Label/Label.tsx | 2 +- .../teams/components/Image/imageStyles.ts | 2 +- .../specs/components/Image/Image-test.tsx | 4 +--- 9 files changed, 9 insertions(+), 30 deletions(-) diff --git a/docs/src/components/ComponentPlayground/componentGenerators.ts b/docs/src/components/ComponentPlayground/componentGenerators.ts index 4565a4b780..766aa87560 100644 --- a/docs/src/components/ComponentPlayground/componentGenerators.ts +++ b/docs/src/components/ComponentPlayground/componentGenerators.ts @@ -71,7 +71,7 @@ export const Icon: KnobComponentGenerators = { }, } -export const Image: KnobComponentGenerators> = { +export const Image: KnobComponentGenerators = { src: ({ propName }) => ({ hook: useStringKnob, name: propName, diff --git a/docs/src/components/Logo/Logo.tsx b/docs/src/components/Logo/Logo.tsx index f7877fe8ac..0c0b026497 100644 --- a/docs/src/components/Logo/Logo.tsx +++ b/docs/src/components/Logo/Logo.tsx @@ -1,7 +1,7 @@ import * as React from 'react' import { Image, ImageProps } from '@fluentui/react' -export type LogoProps = ImageProps & { +export type LogoProps = ImageProps & { flavor?: 'black' | 'white' | 'inverted' } diff --git a/docs/src/examples/components/Image/Types/ImageExample.tsx b/docs/src/examples/components/Image/Types/ImageExample.tsx index fd33bfb394..231d49c840 100644 --- a/docs/src/examples/components/Image/Types/ImageExample.tsx +++ b/docs/src/examples/components/Image/Types/ImageExample.tsx @@ -1,25 +1,6 @@ import * as React from 'react' import { Image } from '@fluentui/react' -const ImageExample = () => ( - <> - {/* Wrong default props */} - {/* */} - {/* */} - - {/* Wrong elements */} - {/* */} - {/* */} - - {/* Wrong attrs on els */} - {/* */} - {/* */} - {/* */} - - {/* Wrong attr on components */} - {/* */} - {/* */} - -) +const ImageExample = () => export default ImageExample diff --git a/packages/react/src/components/Dropdown/DropdownItem.tsx b/packages/react/src/components/Dropdown/DropdownItem.tsx index 0424eca30d..9c5d02a181 100644 --- a/packages/react/src/components/Dropdown/DropdownItem.tsx +++ b/packages/react/src/components/Dropdown/DropdownItem.tsx @@ -44,7 +44,7 @@ export interface DropdownItemProps extends UIComponentProps { header?: ShorthandValue /** Item's image. */ - image?: ShorthandValue> + image?: ShorthandValue /** Indicated whether the item has been set active by keyboard. */ isFromKeyboard?: boolean diff --git a/packages/react/src/components/Dropdown/DropdownSelectedItem.tsx b/packages/react/src/components/Dropdown/DropdownSelectedItem.tsx index b2c28dbf71..1faac19891 100644 --- a/packages/react/src/components/Dropdown/DropdownSelectedItem.tsx +++ b/packages/react/src/components/Dropdown/DropdownSelectedItem.tsx @@ -42,7 +42,7 @@ export interface DropdownSelectedItemProps extends UIComponentProps /** Image of the selected item. */ - image?: ShorthandValue> + image?: ShorthandValue /** * Called on selected item click. diff --git a/packages/react/src/components/Image/Image.tsx b/packages/react/src/components/Image/Image.tsx index 92dfaa6ab6..47684ac611 100644 --- a/packages/react/src/components/Image/Image.tsx +++ b/packages/react/src/components/Image/Image.tsx @@ -45,7 +45,7 @@ export interface ImageOwnProps src?: string } -export type ImageProps = ImageOwnProps & +export type ImageProps = ImageOwnProps & Omit, keyof ImageOwnProps> function Image(props: ImageProps): React.ReactElement { diff --git a/packages/react/src/components/Label/Label.tsx b/packages/react/src/components/Label/Label.tsx index 2fc4de10cc..2e57a1f5af 100644 --- a/packages/react/src/components/Label/Label.tsx +++ b/packages/react/src/components/Label/Label.tsx @@ -60,7 +60,7 @@ export interface LabelProps iconPosition?: 'start' | 'end' /** A Label can contain an image. */ - image?: ShorthandValue> + image?: ShorthandValue /** A Label can position its image at the start or end of the layout. */ imagePosition?: 'start' | 'end' diff --git a/packages/react/src/themes/teams/components/Image/imageStyles.ts b/packages/react/src/themes/teams/components/Image/imageStyles.ts index 00946a890b..f23037f521 100644 --- a/packages/react/src/themes/teams/components/Image/imageStyles.ts +++ b/packages/react/src/themes/teams/components/Image/imageStyles.ts @@ -2,7 +2,7 @@ import { ComponentSlotStylesPrepared, ICSSInJSStyle } from '@fluentui/styles' import { ImageProps } from '../../../../components/Image/Image' import { ImageVariables } from './imageVariables' -export type ImageStylesProps = Pick, 'avatar' | 'circular' | 'fluid'> +export type ImageStylesProps = Pick const imageStyles: ComponentSlotStylesPrepared = { root: ({ props, variables }): ICSSInJSStyle => ({ diff --git a/packages/react/test/specs/components/Image/Image-test.tsx b/packages/react/test/specs/components/Image/Image-test.tsx index ff7c14a120..6d8c7cd8fc 100644 --- a/packages/react/test/specs/components/Image/Image-test.tsx +++ b/packages/react/test/specs/components/Image/Image-test.tsx @@ -5,9 +5,7 @@ import Image from 'src/components/Image/Image' import { mountWithProviderAndGetComponent } from 'test/utils' describe('Image', () => { - isConformant(Image, { - constructorName: 'Image', - }) + isConformant(Image) describe('accessibility', () => { handlesAccessibility(Image, { From 40d952672f16faf1edb21717440112e4e491f25c Mon Sep 17 00:00:00 2001 From: Oleksandr Fediashov Date: Wed, 19 Feb 2020 17:19:33 +0100 Subject: [PATCH 3/6] wip --- packages/react/src/components/Avatar/Avatar.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/components/Avatar/Avatar.tsx b/packages/react/src/components/Avatar/Avatar.tsx index 2a663b6dc2..bf50e2cc18 100644 --- a/packages/react/src/components/Avatar/Avatar.tsx +++ b/packages/react/src/components/Avatar/Avatar.tsx @@ -31,7 +31,7 @@ export interface AvatarProps extends UIComponentProps { accessibility?: Accessibility /** Shorthand for the image. */ - image?: ShorthandValue> + image?: ShorthandValue /** Shorthand for the label. */ label?: ShorthandValue From e5dce0f836ed5175d5988406703f9f63c2ae5b31 Mon Sep 17 00:00:00 2001 From: Oleksandr Fediashov Date: Wed, 19 Feb 2020 17:22:34 +0100 Subject: [PATCH 4/6] cleanup & remove comments --- packages/react/src/components/Image/Image.tsx | 2 +- packages/react/src/types.ts | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/react/src/components/Image/Image.tsx b/packages/react/src/components/Image/Image.tsx index 47684ac611..83f2c37ea3 100644 --- a/packages/react/src/components/Image/Image.tsx +++ b/packages/react/src/components/Image/Image.tsx @@ -45,7 +45,7 @@ export interface ImageOwnProps src?: string } -export type ImageProps = ImageOwnProps & +export type ImageProps = ImageOwnProps & Omit, keyof ImageOwnProps> function Image(props: ImageProps): React.ReactElement { diff --git a/packages/react/src/types.ts b/packages/react/src/types.ts index 7a4aa8b096..650f79c810 100644 --- a/packages/react/src/types.ts +++ b/packages/react/src/types.ts @@ -21,7 +21,6 @@ export type DebounceResultFn = T & { // Source: https://github.com/emotion-js/emotion/blob/master/packages/styled-base/types/helper.d.ts export type PropsOfElement< - // eslint-disable-next-line @typescript-eslint/no-explicit-any E extends keyof JSX.IntrinsicElements | React.JSXElementConstructor > = JSX.LibraryManagedAttributes> From 6a54173b7a73656b86f1aead4b4f456a43fe50e5 Mon Sep 17 00:00:00 2001 From: Oleksandr Fediashov Date: Wed, 19 Feb 2020 17:28:58 +0100 Subject: [PATCH 5/6] move docblock --- packages/react/src/components/Image/Image.tsx | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/react/src/components/Image/Image.tsx b/packages/react/src/components/Image/Image.tsx index 83f2c37ea3..26bd318b6e 100644 --- a/packages/react/src/components/Image/Image.tsx +++ b/packages/react/src/components/Image/Image.tsx @@ -48,6 +48,17 @@ export interface ImageOwnProps export type ImageProps = ImageOwnProps & Omit, keyof ImageOwnProps> +/** + * An Image is a graphic representation of something. + * + * @accessibility + * If image should be visible to screen readers, textual representation needs to be provided in 'alt' property. + * + * Other considerations: + * - when alt property is empty, then Narrator in scan mode navigates to image and narrates it as empty paragraph. + * - when image has role='presentation' then screen readers navigate to the element in scan/virtual mode. To avoid this, the attribute "aria-hidden='true'" is applied by the default image behavior. + * - when alt property is used in combination with aria-label, arialabbeledby or title, additional screen readers verification is needed as each screen reader handles this combination differently. + */ function Image(props: ImageProps): React.ReactElement { const context: ProviderContextPrepared = React.useContext(ThemeContext) const { setStart, setEnd } = useTelemetry(Image.displayName, context.telemetry) @@ -122,15 +133,4 @@ Image.handledProps = Object.keys(Image.propTypes) as any Image.create = createShorthandFactory({ Component: Image, mappedProp: 'src', allowsJSX: false }) -/** - * An Image is a graphic representation of something. - * - * @accessibility - * If image should be visible to screen readers, textual representation needs to be provided in 'alt' property. - * - * Other considerations: - * - when alt property is empty, then Narrator in scan mode navigates to image and narrates it as empty paragraph. - * - when image has role='presentation' then screen readers navigate to the element in scan/virtual mode. To avoid this, the attribute "aria-hidden='true'" is applied by the default image behavior. - * - when alt property is used in combination with aria-label, arialabbeledby or title, additional screen readers verification is needed as each screen reader handles this combination differently. - */ export default Image From 97a7af890ab3f7911ee1f2ebdfae6810f2862624 Mon Sep 17 00:00:00 2001 From: Oleksandr Fediashov Date: Wed, 19 Feb 2020 17:48:48 +0100 Subject: [PATCH 6/6] fix typings --- packages/react/src/components/Image/Image.tsx | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/react/src/components/Image/Image.tsx b/packages/react/src/components/Image/Image.tsx index 26bd318b6e..cd5aa65f70 100644 --- a/packages/react/src/components/Image/Image.tsx +++ b/packages/react/src/components/Image/Image.tsx @@ -19,7 +19,7 @@ import { ThemeContext } from 'react-fela' import { createShorthandFactory, UIComponentProps, commonPropTypes } from '../../utils' import { PropsOfElement, ProviderContextPrepared } from '../../types' -export interface ImageOwnProps +export interface ImageOwnProps extends UIComponentProps, ImageBehaviorProps { as?: E @@ -45,8 +45,10 @@ export interface ImageOwnProps src?: string } -export type ImageProps = ImageOwnProps & - Omit, keyof ImageOwnProps> +export type ImageStrictProps = ImageOwnProps & + Omit, keyof ImageOwnProps> + +export type ImageProps = ImageStrictProps /** * An Image is a graphic representation of something. @@ -59,7 +61,9 @@ export type ImageProps = ImageO * - when image has role='presentation' then screen readers navigate to the element in scan/virtual mode. To avoid this, the attribute "aria-hidden='true'" is applied by the default image behavior. * - when alt property is used in combination with aria-label, arialabbeledby or title, additional screen readers verification is needed as each screen reader handles this combination differently. */ -function Image(props: ImageProps): React.ReactElement { +function Image( + props: ImageStrictProps, +): React.ReactElement { const context: ProviderContextPrepared = React.useContext(ThemeContext) const { setStart, setEnd } = useTelemetry(Image.displayName, context.telemetry) setStart()