Repository: uber-web/baseui
Pull request #238 (2018-09-13 16:13:20)

docs(file-uploader): file uploader rfc
<img width="656" alt="screen shot 2018-09-13 at 10 11 57 am" src="https://user-images.githubusercontent.com/1920399/45512880-ecef2980-b755-11e8-8be3-19b7d2572e8e.png">



Patch:
diff --git a/package.json b/package.json
index 45d413fe7..ae73e21e5 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "baseui",
-  "version": "1.7.1",
+  "version": "1.7.3",
   "description": "React implementation of uber design components platform.",
   "keywords": [
     "react",
@@ -22,10 +22,11 @@
     "precommit": "pretty-quick --staged && yarn run test",
     "prebuild": "rimraf dist",
     "build:copy-files": "cp package.json dist/package.json && cp README.md dist/README.md",
+    "build:copy-flow-files": "./scripts/flow-copy-src.js",
     "build:es2015": "cross-env NODE_ENV=production babel ./src --out-dir ./dist --ignore *.test.js",
     "build:es2015modules": "cross-env NODE_ENV=production BABEL_ENV=modules babel ./src/index.js --out-file ./dist/index.es.js",
     "build:es": "cross-env NODE_ENV=production BABEL_ENV=es babel ./src --out-dir ./dist/es --ignore *.test.js",
-    "build": "yarn build:es2015 && yarn build:es2015modules && yarn build:es && yarn build:copy-files",
+    "build": "yarn build:es2015 && yarn build:es2015modules && yarn build:es && yarn build:copy-files && yarn build:copy-flow-files",
     "unit-test": "yarn jest --coverage",
     "e2e": "node ./e2e/e2e.js",
     "e2e:build": "webpack --config webpack.e2e.config.js --mode=production",
diff --git a/rfcs/file-uploader-component.md b/rfcs/file-uploader-component.md
new file mode 100644
index 000000000..f1b794d72
--- /dev/null
+++ b/rfcs/file-uploader-component.md
@@ -0,0 +1,88 @@
+# File Uploader Component
+
+A component to upload a selected or gragged file or multiple files.
+
+## Usage
+
+### Basic usage
+
+```javascript
+import * as React from 'react';
+import {FileUploader} from 'baseui/file-uploader';
+
+export default () => <FileUploader action={url} />;
+```
+
+### Advanced usage
+
+```javascript
+import * as React from 'react';
+import {FileUploader} from 'baseui/file-uploader';
+
+export default () => {
+  return
+    <FileUploader
+      action={url}
+      onChange={(file)=>{console.log(`File ${file.name} have been selected`)}}
+      overrides={{
+        FileSelect: props => <CustomButton>Click me</CustomButton>,
+      }}
+    />;
+}
+```
+
+## Exports
+
+* `FileUploader`
+* `StyledRoot`
+* `StyledContentContainer`
+* `StyledFileDragAndDrop`
+* `StyledFileSelectButton`
+* `StyledInput`
+
+## `FileUploader` API
+
+* `accept: ?string` - Optional, Defaults to `null`
+  Defines the types of files that can be selected. See [input accept attribute](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input#attr-accept)
+* `action: string | function: () => string` - Optional | Required, Defaults to ''
+  Sets the uploading URL. Is Required when `autoUpload` is set to `true`
+* `autoUpload: boolean` - Optional, Defaults to `true`
+  Defines if a selected file should start uploading automatically
+* `disabled: boolean` - Optional, Defaults to `false`
+  Defines if the control is disabled
+* `multi: boolean` - Optional, Defaults to `false`
+  Defines if multiple files can be selected
+* `onChange: function` - Optional
+  A function called when a file is selected
+* `onError: function` - Optional, Defaults to `false`
+  A function called when a file upload failed
+* `onFileUploaded: function` - Optional
+  A function called when file is done uploading. It's not called if autoUpload is set to `false`
+* `onFileRemove: function` - Optional
+  A function called on a remove file action
+`overrides: {Root, ContentContainer, FileDragAndDrop, FileSelectButton, Input}` - Optional
+  Overrides for presentational components. See "Presentational Components Props API" below.
+  * `[ComponentName]: ReactComponent | {props: {}, style: {}, component: ReactComponent}` - Optional
+* `request: {data: {} | function, headers: {}}` - Optional
+  Sets uploading params, headers
+
+## Presentational components props API
+
+These properties are passed to every presentational (styled) component that is exported:
+
+* `$disabled: boolean`
+* `$multi: boolean`
+* `$isFocused: boolean`
+* `$error: boolean`
+* `$theme: theme`
+
+## Dependencies
+
+Does this component depend on any 3rd party packages or other internal components?
+
+## Accessibility
+
+* The focusable "Browse files" label will have `role="button"` set
+* "Browse files" label will also have `aria-controls="fileupload"` set
+* For the keyboard support and navigation "Browse files" needs distinctive focus styles and `cursor: pointer` on hover.
+* Show only "Browse files" label when drag & drop file upload is not supported.
diff --git a/scripts/flow-copy-src.js b/scripts/flow-copy-src.js
old mode 100644
new mode 100755
index d586f2534..36d4fe083
--- a/scripts/flow-copy-src.js
+++ b/scripts/flow-copy-src.js
@@ -1,21 +1,28 @@
+#!/usr/bin/env node
+
 /* eslint-env node */
 // @flow
 
 const flowCopySource = require('flow-copy-source');
+const path = require('path');
 
 async function run() {
-  await flowCopySource([`${__dirname}/../src`], `${__dirname}/../dist/src`, {
-    ignore: [
-      '**/*.test.js',
-      '**/*.setup.js',
-      '**/*stories.js',
-      'test/**/*.js',
-      '**/__mocks__/*.js',
-      '**/e2e.js',
-      'coverage/**/*.js',
-      '**/*examples.js',
-    ],
-  });
+  await flowCopySource(
+    [path.resolve(__dirname, '../src')],
+    path.resolve(__dirname, '../dist'),
+    {
+      ignore: [
+        '**/*.test.js',
+        '**/*.setup.js',
+        '**/*stories.js',
+        'test/**/*.js',
+        '**/__mocks__/*.js',
+        '**/e2e.js',
+        'coverage/**/*.js',
+        '**/*examples.js',
+      ],
+    },
+  );
 }
 
 run();
diff --git a/src/form-control/form-control.js b/src/form-control/form-control.js
index 96232b6bc..ae3fe15ea 100644
--- a/src/form-control/form-control.js
+++ b/src/form-control/form-control.js
@@ -14,7 +14,7 @@ import {
 } from './styled-components';
 import type {FormControlPropsT} from './types';
 
-function mapStyletronProps(props: {}, mapper: {}) {
+function getSharedProps(props: {}, mapper: {}) {
   return Object.keys(props).reduce((newProps, propName) => {
     const newName = mapper[propName] && `$${propName}`;
     if (newName) {
@@ -42,10 +42,7 @@ export default class FormControl extends React.Component<FormControlPropsT> {
     } = this.props;
 
     const onlyChildProps = React.Children.only(children).props;
-    const sharedProps = mapStyletronProps(
-      onlyChildProps,
-      STYLETRON_PROP_MAPPER,
-    );
+    const sharedProps = getSharedProps(onlyChildProps, STYLETRON_PROP_MAPPER);
     sharedProps.$error = this.props.error || sharedProps.$error;
     const Label = getOverride(LabelOverride) || StyledLabel;
     const Caption = getOverride(CaptionOverride) || StyledCaption;
diff --git a/src/menu/constants.js b/src/menu/constants.js
index 8d4e50e19..27fa62807 100644
--- a/src/menu/constants.js
+++ b/src/menu/constants.js
@@ -13,7 +13,7 @@ export const STATE_CHANGE_TYPES = {
 };
 
 // Dict of props to prepend $ to pass thru to styletron
-export const STYLETRON_PROP_MAPPER = {
+export const SHARED_PROPS_MAPPER = {
   ref: true,
   isHighlighted: true,
 };
diff --git a/src/menu/menu.js b/src/menu/menu.js
index 5c1956250..b47f0b2fc 100644
--- a/src/menu/menu.js
+++ b/src/menu/menu.js
@@ -9,14 +9,14 @@ import * as React from 'react';
 // Components
 import {List as StyledList} from './styled-components';
 import OptionList from './option-list';
-import {mapStyletronProps} from './utils';
+import {getSharedProps} from './utils';
 import {getOverrideObject} from '../helpers/overrides';
 // Types
 import type {StatelessMenuPropsT} from './types';
 
 export default function Menu({
   items,
-  getRequiredItemProps = (item, index) => ({key: String(index)}),
+  getRequiredItemProps = (item, index) => ({}),
   rootRef = React.createRef(),
   overrides = {},
 }: StatelessMenuPropsT) {
@@ -32,12 +32,14 @@ export default function Menu({
     <List $ref={rootRef} {...listProps}>
       {items.map((item, index) => {
         const requiredProps = getRequiredItemProps(item, index);
-        // Need to be explicit with `key` otherwise eslint throws error?
         return (
-          // eslint-disable-next-line react/jsx-key
           <Option
+            key={index}
             item={item}
-            {...mapStyletronProps(requiredProps)}
+            role="option"
+            // Allows tab focus into first element
+            tabIndex={index === 0 ? 0 : -1}
+            {...getSharedProps(requiredProps)}
             {...optionProps}
           />
         );
diff --git a/src/menu/stateful-container.js b/src/menu/stateful-container.js
index c88d9f16c..063bc22a0 100644
--- a/src/menu/stateful-container.js
+++ b/src/menu/stateful-container.js
@@ -12,32 +12,35 @@ import {scrollItemIntoView} from './utils';
 // Types
 import type {
   StatefulContainerPropsT,
-  DefaultStatefulContainerPropsT,
   StatefulContainerStateT,
   GetRequiredItemPropsFnT,
   RootRefT,
   RenderPropsT,
+  StateReducerFnT,
 } from './types';
 
 export default class MenuStatefulContainer extends React.Component<
   StatefulContainerPropsT,
   StatefulContainerStateT,
 > {
-  static defaultProps: DefaultStatefulContainerPropsT = {
+  static defaultProps = {
     initialState: {
       // We start the index at -1 to indicate that no highlighting exists initially
       highlightedIndex: -1,
     },
-    stateReducer: (changeType, changes) => changes,
+    stateReducer: (
+      changeType: ?$PropertyType<StateReducerFnT, 'changeType'>,
+      changes: $PropertyType<StateReducerFnT, 'changes'>,
+    ) => changes,
     onItemSelect: () => {},
-    children: () => {},
+    children: () => null,
   };
 
   state: StatefulContainerStateT = {...this.props.initialState};
 
   componentDidMount() {
     if (__BROWSER__) {
-      // TODO: perhaps only bind event listener on focus
+      // TODO(#185): perhaps only bind event listener on focus
       document.addEventListener('keydown', this.onKeyDown);
     }
   }
@@ -61,7 +64,6 @@ export default class MenuStatefulContainer extends React.Component<
     changes: StatefulContainerStateT,
   ) {
     const {stateReducer} = this.props;
-    // $FlowFixMe
     this.setState(stateReducer(changeType, changes, this.state));
   }
 
@@ -127,11 +129,9 @@ export default class MenuStatefulContainer extends React.Component<
       this.refList[index] = itemRef;
     }
     return {
-      key: `${index}`,
       ref: itemRef,
       isHighlighted: highlightedIndex === index,
       onClick: onClickHandler,
-      role: 'option',
       'aria-activedescendant': highlightedIndex === index,
     };
   };
@@ -139,7 +139,6 @@ export default class MenuStatefulContainer extends React.Component<
   render() {
     const {highlightedIndex} = this.state;
     const {children, items} = this.props;
-    // $FlowFixMe
     return children(
       ({
         highlightedIndex,
diff --git a/src/menu/stateful-menu.js b/src/menu/stateful-menu.js
index 7d8e1baf6..b0e4f8e18 100644
--- a/src/menu/stateful-menu.js
+++ b/src/menu/stateful-menu.js
@@ -17,6 +17,8 @@ export default class StatefulMenu extends React.PureComponent<
   StatefulMenuPropsT,
 > {
   static defaultProps = {
+    // Mostly to satisfy flow
+    ...StatefulContainer.defaultProps,
     overrides: {},
   };
 
diff --git a/src/menu/styled-components.js b/src/menu/styled-components.js
index 05e9b2516..f101a7923 100644
--- a/src/menu/styled-components.js
+++ b/src/menu/styled-components.js
@@ -24,6 +24,7 @@ export const List = styled('ul', ({$theme}: StyledPropsT) => ({
   background: $theme.colors.white,
   borderRadius: $theme.borders.radius300,
   boxShadow: $theme.lighting.shadow600,
+  overflow: 'auto',
 }));
 
 export const ListItem = styled(
diff --git a/src/menu/types.js b/src/menu/types.js
index cfc8d37d6..53c50634f 100644
--- a/src/menu/types.js
+++ b/src/menu/types.js
@@ -10,36 +10,34 @@ import * as React from 'react';
 import type {OverrideT} from '../helpers/overrides';
 import {STATE_CHANGE_TYPES} from './constants';
 
-export type ItemsT = Array<*>;
+export type ItemT = *;
+export type ItemsT = Array<ItemT>;
 
-export type GetItemLabelFnT = (item: *) => React$Node;
+export type GetItemLabelFnT = (item: ItemT) => React$Node;
 
 export type GetProfileItemLabelsFnT = (
-  item: *,
+  item: ItemT,
 ) => {
   title?: string,
   subtitle?: string,
   body?: string,
 };
 
-export type GetProfileItemImgFnT = (item: *) => string | React.ComponentType<*>;
+export type GetProfileItemImgFnT = (
+  item: ItemT,
+) => string | React.ComponentType<*>;
 
-export type GetProfileItemImgTextFnT = (item: *) => string;
+export type GetProfileItemImgTextFnT = (item: ItemT) => string;
 
 export type SetRootRefFnT = (ref: React$ElementRef<*>) => void;
 
 export type RootRefT = React$ElementRef<*>;
 
 export type OnItemSelectFnT = (
-  item: ?{},
+  item: ?ItemT,
   event: SyntheticEvent<> | KeyboardEvent,
 ) => mixed;
 
-export type OverridesT = {
-  List?: OverrideT<*>,
-  ListItem?: OverrideT<*>,
-};
-
 export type ProfileOverridesT = {
   List?: OverrideT<*>,
   ListItemProfile?: OverrideT<*>,
@@ -52,22 +50,19 @@ export type ProfileOverridesT = {
 };
 
 export type RenderItemPropsT = {
-  key: string,
   ref?: React$ElementRef<*>,
   // indicates when the item is visually focused
   isHighlighted?: boolean,
-  role?: string,
-  // indicates when the item is visually focused
   'aria-activedescendant'?: boolean,
 };
 
 export type GetRequiredItemPropsFnT = (
-  item: {},
+  item: ItemT,
   index: number,
 ) => RenderItemPropsT;
 
 export type StateReducerFnT = (
-  changeType: $Keys<typeof STATE_CHANGE_TYPES>,
+  changeType: ?$Keys<typeof STATE_CHANGE_TYPES>,
   changes: StatefulContainerStateT,
   currentState: StatefulContainerStateT,
 ) => StatefulContainerStateT;
@@ -99,10 +94,10 @@ export type DefaultStatefulContainerPropsT = {
 
 export type StatefulContainerPropsT = {
   items: ItemsT,
-  initialState?: StatefulContainerStateT,
-  stateReducer?: StateReducerFnT,
-  onItemSelect?: OnItemSelectFnT,
-  children?: RenderPropsT => React.Node,
+  initialState: StatefulContainerStateT,
+  stateReducer: StateReducerFnT,
+  onItemSelect: OnItemSelectFnT,
+  children: RenderPropsT => React.Node,
 };
 
 export type MenuPropsT = {
@@ -137,7 +132,7 @@ export type StatelessMenuProfilePropsT = SharedStatelessPropsT &
   MenuProfilePropsT;
 
 export type OptionListPropsT = {
-  item: *,
+  item: ItemT,
   getItemLabel: GetItemLabelFnT,
   overrides?: {
     ListItem?: OverrideT<*>,
@@ -145,7 +140,7 @@ export type OptionListPropsT = {
 };
 
 export type OptionProfilePropsT = {
-  item: *,
+  item: ItemT,
   getProfileItemLabels: GetProfileItemLabelsFnT,
   getProfileItemImg: GetProfileItemImgFnT,
   getProfileItemImgText: GetProfileItemImgTextFnT,
diff --git a/src/menu/utils.js b/src/menu/utils.js
index 33345e372..3fd322250 100644
--- a/src/menu/utils.js
+++ b/src/menu/utils.js
@@ -7,7 +7,7 @@ LICENSE file in the root directory of this source tree.
 // @flow
 /* eslint-disable import/prefer-default-export */
 import smoothscroll from 'smoothscroll-polyfill';
-import {STYLETRON_PROP_MAPPER} from './constants';
+import {SHARED_PROPS_MAPPER} from './constants';
 
 if (__BROWSER__) {
   smoothscroll.polyfill();
@@ -18,9 +18,9 @@ if (__BROWSER__) {
  * all of the existing prop keys inside mapper with $ to present styletron
  * from passing through those props to the underlying React component.
  */
-export function mapStyletronProps(
+export function getSharedProps(
   props: {},
-  mapper: {} = STYLETRON_PROP_MAPPER,
+  mapper: {} = SHARED_PROPS_MAPPER,
 ): {} {
   return Object.keys(props).reduce((newProps, propName) => {
     const newName = mapper[propName] ? `$${propName}` : propName;


Test patch:
diff --git a/src/menu/__tests__/__snapshots__/styled-components.test.js.snap b/src/menu/__tests__/__snapshots__/styled-components.test.js.snap
index 1e3e72b0a..e8de604f3 100644
--- a/src/menu/__tests__/__snapshots__/styled-components.test.js.snap
+++ b/src/menu/__tests__/__snapshots__/styled-components.test.js.snap
@@ -6,6 +6,7 @@ Object {
   "borderRadius": "$theme.borders.radius300",
   "boxShadow": "$theme.lighting.shadow600",
   "margin": 0,
+  "overflow": "auto",
   "padding": 0,
   "position": "relative",
 }
diff --git a/src/menu/__tests__/stateful-container.test.js b/src/menu/__tests__/stateful-container.test.js
index 2ac21db05..73dcf3235 100644
--- a/src/menu/__tests__/stateful-container.test.js
+++ b/src/menu/__tests__/stateful-container.test.js
@@ -91,11 +91,9 @@ describe('Menu StatefulContainer', () => {
     const item = mockItems[0];
     const props = component.instance().getRequiredItemProps(item, 0);
     expect(props).toEqual({
-      key: '0',
       ref: React.createRef(),
       isHighlighted: false,
       onClick: props.onClick,
-      role: 'option',
       'aria-activedescendant': false,
     });
     props.onClick();
@@ -110,11 +108,9 @@ describe('Menu StatefulContainer', () => {
     const item = mockItems[0];
     const props = component.instance().getRequiredItemProps(item, 0);
     expect(props).toEqual({
-      key: '0',
       ref: React.createRef(),
       isHighlighted: true,
       onClick: props.onClick,
-      role: 'option',
       'aria-activedescendant': true,
     });
   });
diff --git a/src/menu/__tests__/utils.test.js b/src/menu/__tests__/utils.test.js
index 35448791e..100e7a9f4 100644
--- a/src/menu/__tests__/utils.test.js
+++ b/src/menu/__tests__/utils.test.js
@@ -7,7 +7,7 @@ LICENSE file in the root directory of this source tree.
 // @flow
 import * as Utils from '../utils';
 
-describe('Menu Utils - mapStyletronProps', () => {
+describe('Menu Utils - getSharedProps', () => {
   test('successfully maps', () => {
     const props = {
       someStyletronProps: 'styletron',
@@ -16,7 +16,7 @@ describe('Menu Utils - mapStyletronProps', () => {
     const mapper = {
       someStyletronProps: true,
     };
-    expect(Utils.mapStyletronProps(props, mapper)).toEqual({
+    expect(Utils.getSharedProps(props, mapper)).toEqual({
       $someStyletronProps: 'styletron',
       normalProps: 'normal',
     });