From 9b798c015e10b26e5cf9f5b3f440998749481481 Mon Sep 17 00:00:00 2001 From: Elizabeth Letourneau Date: Wed, 6 Mar 2019 16:22:08 -0500 Subject: [PATCH 1/2] Allow fixed height on Dropzone --- UNRELEASED.md | 1 + src/components/DropZone/DropZone.scss | 11 ++ src/components/DropZone/DropZone.tsx | 164 +++++++++++------- .../DropZone/components/Context/Context.tsx | 5 +- .../components/FileUpload/FileUpload.tsx | 117 ++++++------- .../FileUpload/tests/FileUpload.test.tsx | 55 +++++- .../DropZone/tests/DropZone.test.tsx | 53 +++--- src/components/DropZone/types.ts | 9 +- 8 files changed, 260 insertions(+), 155 deletions(-) diff --git a/UNRELEASED.md b/UNRELEASED.md index ed213280bb4..15b8ef93867 100644 --- a/UNRELEASED.md +++ b/UNRELEASED.md @@ -16,6 +16,7 @@ Use [the changelog guidelines](https://git.io/polaris-changelog-guidelines) to f - Fixed accessibility issue with ChoiceList errors not being correctly connected to the inputs ([#1824](https://github.com/Shopify/polaris-react/pull/1824)); - Fixed `Tab` `aria-controls` pointing to a non-existent `Panel` `id` ([#1869](https://github.com/Shopify/polaris-react/pull/1869)) +- Constrained `DropZone` height based on inherited wrapper height ([#1138](https://github.com/Shopify/polaris-react/pull/1138)) ### Documentation diff --git a/src/components/DropZone/DropZone.scss b/src/components/DropZone/DropZone.scss index 098aee94a2b..ded01f20b43 100755 --- a/src/components/DropZone/DropZone.scss +++ b/src/components/DropZone/DropZone.scss @@ -17,12 +17,17 @@ $dropzone-overlay-border-color-error: color('red'); $dropzone-overlay-background: color('indigo', 'lighter'); $dropzone-overlay-background-error: color('red', 'lighter'); +.DropZoneWrapper { + height: inherit; +} + .DropZone { position: relative; display: flex; justify-content: center; background-color: $dropzone-background; border-radius: $dropzone-border-radius; + height: inherit; } .hasOutline { @@ -38,6 +43,10 @@ $dropzone-overlay-background-error: color('red', 'lighter'); cursor: not-allowed; } +.isMeasuring { + visibility: hidden; +} + .sizeExtraLarge { min-height: $dropzone-min-height-extra-large; } @@ -58,7 +67,9 @@ $dropzone-overlay-background-error: color('red', 'lighter'); } .Container { + display: flex; flex: 1; + justify-content: center; } .Overlay { diff --git a/src/components/DropZone/DropZone.tsx b/src/components/DropZone/DropZone.tsx index 9309fc858cf..d392705d72a 100755 --- a/src/components/DropZone/DropZone.tsx +++ b/src/components/DropZone/DropZone.tsx @@ -23,7 +23,7 @@ import {withAppProvider, WithAppProviderProps} from '../AppProvider'; import {FileUpload, Provider} from './components'; import {fileAccepted, getDataTransferFiles} from './utils'; -import {DropZoneContext} from './types'; +import {DropZoneContext, Size} from './types'; import styles from './DropZone.scss'; @@ -31,13 +31,15 @@ export type Type = 'file' | 'image'; export interface State { id: string; - size: string; + height: Size; + width: Size; type?: string; error?: boolean; dragging: boolean; overlayText?: string; errorOverlayText?: string; numFiles: number; + measuring: boolean; } export interface Props { @@ -163,18 +165,10 @@ class DropZone extends React.Component { return; } - let size = 'extraLarge'; - const width = this.node.current.getBoundingClientRect().width; + const height = this.setWrapperSize('height', this.node); + const width = this.setWrapperSize('width', this.node); - if (width < 100) { - size = 'small'; - } else if (width < 160) { - size = 'medium'; - } else if (width < 300) { - size = 'large'; - } - - this.setState({size}); + this.setState({height, width, measuring: false}); }, 50, {trailing: true}, @@ -195,19 +189,23 @@ class DropZone extends React.Component { this.state = { type, id: props.id || getUniqueID(), - size: 'extraLarge', + height: Size.ExtraLarge, dragging: false, error: false, overlayText: translate(`Polaris.DropZone.overlayText${suffix}`), errorOverlayText: translate(`Polaris.DropZone.errorOverlayText${suffix}`), numFiles: 0, + width: Size.ExtraLarge, + measuring: true, }; } get getContext(): DropZoneContext { + const {width, height, type = 'file'} = this.state; return { - size: this.state.size, - type: this.state.type || 'file', + width, + height, + type, }; } @@ -220,9 +218,11 @@ class DropZone extends React.Component { id, dragging, error, - size, + height, + width, overlayText, errorOverlayText, + measuring, } = this.state; const { label, @@ -244,6 +244,7 @@ class DropZone extends React.Component { disabled, type: 'file', multiple: allowMultiple, + name: id, ref: this.fileInputNode, onChange: this.handleDrop, autoComplete: 'off', @@ -253,11 +254,28 @@ class DropZone extends React.Component { styles.DropZone, outline && styles.hasOutline, (active || dragging) && styles.isDragging, + measuring && styles.isMeasuring, error && styles.hasError, - size && size === 'extraLarge' && styles.sizeExtraLarge, - size && size === 'large' && styles.sizeLarge, - size && size === 'medium' && styles.sizeMedium, - size && size === 'small' && styles.sizeSmall, + height === Size.ExtraLarge && styles.sizeExtraLarge, + height === Size.Large && styles.sizeLarge, + height === Size.Medium && styles.sizeMedium, + height === Size.Small && styles.sizeSmall, + ); + + const extraLargeDropZoneSize = + width === Size.ExtraLarge && height !== Size.Small; + + const mediumLargeDropZoneSize = + (width === Size.Medium || width === Size.Large) && height !== Size.Small; + + const dragOverlayDisplayText = extraLargeDropZoneSize && ( + + {overlayText} + + ); + + const dragOverlayCaption = mediumLargeDropZoneSize && ( + {overlayText} ); const dragOverlay = @@ -265,31 +283,29 @@ class DropZone extends React.Component {
- {size === 'extraLarge' && ( - - {overlayText} - - )} - {(size === 'medium' || size === 'large') && ( - {overlayText} - )} + {dragOverlayDisplayText} + {dragOverlayCaption}
) : null; + const dragErrorOverlayDisplayText = extraLargeDropZoneSize && ( + + {errorOverlayText} + + ); + + const dragErrorOverlayCaption = mediumLargeDropZoneSize && ( + {errorOverlayText} + ); + const dragErrorOverlay = dragging && error ? (
- {size === 'extraLarge' && ( - - {errorOverlayText} - - )} - {(size === 'medium' || size === 'large') && ( - {errorOverlayText} - )} + {dragErrorOverlayDisplayText} + {dragErrorOverlayCaption}
) : null; @@ -298,31 +314,39 @@ class DropZone extends React.Component { ? label : intl.translate('Polaris.DropZone.FileUpload.label'); const labelHiddenValue = label ? labelHidden : true; + const dropZoneMarkup = ( +
+
+ {dragOverlay} + {dragErrorOverlay} +
{children}
+ + + +
+
+ ); + + const labelledDropzoneMarkup = label ? ( + + {dropZoneMarkup} + + ) : ( + dropZoneMarkup + ); return ( - - -
- {dragOverlay} - {dragErrorOverlay} -
{children}
- - - -
- - + {labelledDropzoneMarkup} ); } @@ -382,6 +406,26 @@ class DropZone extends React.Component { this.fileInputNode.current.click(); }; + private setWrapperSize = ( + size: string, + node: React.RefObject, + ) => { + let wrapperSize = Size.ExtraLarge; + + if (node.current) { + const getSize = size === 'height' ? 'height' : 'width'; + const wrapper = node.current.getBoundingClientRect()[getSize]; + if (wrapper < Size.Small) { + wrapperSize = Size.Small; + } else if (wrapper < Size.Medium) { + wrapperSize = Size.Medium; + } else if (wrapper < Size.Large) { + wrapperSize = Size.Large; + } + } + return wrapperSize; + }; + private getValidatedFiles = (files: File[] | DataTransferItem[]) => { const {accept, allowMultiple, customValidator} = this.props; diff --git a/src/components/DropZone/components/Context/Context.tsx b/src/components/DropZone/components/Context/Context.tsx index 2143e1b2ebb..d0f4b5082de 100644 --- a/src/components/DropZone/components/Context/Context.tsx +++ b/src/components/DropZone/components/Context/Context.tsx @@ -1,8 +1,9 @@ import * as React from 'react'; -import {DropZoneContext} from '../../types'; +import {DropZoneContext, Size} from '../../types'; const {Provider, Consumer} = React.createContext({ - size: 'extraLarge', + width: Size.ExtraLarge, + height: Size.ExtraLarge, type: 'file', }); diff --git a/src/components/DropZone/components/FileUpload/FileUpload.tsx b/src/components/DropZone/components/FileUpload/FileUpload.tsx index 6f60d8ac603..e25aedb251d 100755 --- a/src/components/DropZone/components/FileUpload/FileUpload.tsx +++ b/src/components/DropZone/components/FileUpload/FileUpload.tsx @@ -16,7 +16,7 @@ import TextStyle from '../../../TextStyle'; import withContext from '../../../WithContext'; import withRef from '../../../WithRef'; -import {DropZoneContext} from '../../types'; +import {DropZoneContext, Size} from '../../types'; import {fileUpload, imageUpload} from '../../images'; import {Consumer} from '../Context'; @@ -91,71 +91,72 @@ class FileUpload extends React.Component { } render() { + const fileUploadMarkup = this.getFileUploadMarkup(); + + return
{fileUploadMarkup}
; + } + + private getFileUploadMarkup() { const { - context: {size, type}, + context: {width, height, type}, } = this.props; + const {actionTitle, actionHint} = this.state; const imageClasses = classNames( styles.Image, - size && size === 'extraLarge' && styles.sizeExtraLarge, - size && size === 'large' && styles.sizeLarge, + width === Size.ExtraLarge && styles.sizeExtraLarge, + width === Size.Large && styles.sizeLarge, ); - const extraLargeView = - size === 'extraLarge' ? ( - - {type === 'file' && ( - - )} - {type === 'image' && ( - - )} - - {actionHint} - - ) : null; - - const largeView = - size === 'large' ? ( - - {type === 'file' && ( - - )} - {type === 'image' && ( - - )} - - + const fileType = type === 'file'; + const imageType = type === 'image'; + + const size = Math.min(width, height); + switch (size) { + case Size.ExtraLarge: + return ( + + {fileType && ( + + )} + {imageType && ( + + )} + {actionHint} - - - ) : null; - - const mediumView = - size === 'medium' ? ( - - {actionTitle} - - {actionHint} - - - ) : null; - - const smallView = - size === 'small' ? ( - - - - ) : null; - - return ( -
- {smallView} - {mediumView} - {largeView} - {extraLargeView} -
- ); + + ); + case Size.Large: + return ( + + {fileType && ( + + )} + {imageType && ( + + )} + + + {actionHint} + + + ); + case Size.Medium: + return ( + + {actionTitle} + + {actionHint} + + + ); + case Size.Small: + return ( + + + + ); + } } } diff --git a/src/components/DropZone/components/FileUpload/tests/FileUpload.test.tsx b/src/components/DropZone/components/FileUpload/tests/FileUpload.test.tsx index 8eee9362598..3d57ee6274c 100755 --- a/src/components/DropZone/components/FileUpload/tests/FileUpload.test.tsx +++ b/src/components/DropZone/components/FileUpload/tests/FileUpload.test.tsx @@ -4,12 +4,19 @@ import {mountWithAppProvider} from 'test-utilities'; import {Provider} from '../../Context'; import FileUpload from '../FileUpload'; import {fileUpload as fileUploadImage, imageUpload} from '../../../images'; +import {Size} from '../../../types'; describe('', () => { describe('extraLarge', () => { it('renders extra large view for type file', () => { const fileUpload = mountWithAppProvider( - + , ); @@ -21,7 +28,13 @@ describe('', () => { it('renders extra large view for type image', () => { const fileUpload = mountWithAppProvider( - + , ); @@ -33,9 +46,22 @@ describe('', () => { }); describe('large', () => { + it('renders large view', () => { + const fileUpload = mountWithAppProvider( + + + , + ); + + expect(fileUpload.find('img').prop('src')).toBe(fileUploadImage); + expect(fileUpload.find(Button)).toHaveLength(1); + expect(fileUpload.find(TextStyle)).toHaveLength(1); + expect(fileUpload.find(Caption)).toHaveLength(1); + }); + it('renders large view for type file', () => { const fileUpload = mountWithAppProvider( - + , ); @@ -48,7 +74,9 @@ describe('', () => { it('renders large view for type image', () => { const fileUpload = mountWithAppProvider( - + , ); @@ -62,7 +90,7 @@ describe('', () => { it('renders medium view', () => { const fileUpload = mountWithAppProvider( - + , ); @@ -73,7 +101,7 @@ describe('', () => { it('renders small view', () => { const fileUpload = mountWithAppProvider( - + , ); @@ -81,9 +109,18 @@ describe('', () => { expect(fileUpload.find(Icon)).toHaveLength(1); }); + it('renders the smaller view when width is small but height is large', () => { + const fileUpload = mountWithAppProvider( + + + , + ); + expect(fileUpload.find(Icon)).toHaveLength(1); + }); + it('sets a default actionTitle if the prop is provided then removed', () => { const fileUpload = mountWithAppProvider( - + , ); @@ -94,7 +131,7 @@ describe('', () => { it('sets a default actionHint if the prop is provided then removed', () => { const fileUpload = mountWithAppProvider( - + , ); @@ -107,7 +144,7 @@ describe('', () => { const actionTitle = 'Add file title'; const actionHint = 'or drop files to upload hint'; const fileUpload = mountWithAppProvider( - + , ); diff --git a/src/components/DropZone/tests/DropZone.test.tsx b/src/components/DropZone/tests/DropZone.test.tsx index 0de0a23a0a8..fe6e7ff9a00 100755 --- a/src/components/DropZone/tests/DropZone.test.tsx +++ b/src/components/DropZone/tests/DropZone.test.tsx @@ -28,12 +28,13 @@ const duplicateFiles = [ const acceptedFiles = [files[0]]; const rejectedFiles = [files[1]]; const origGetBoundingClientRect = Element.prototype.getBoundingClientRect; -const widths = { - small: 99, - medium: 159, - large: 299, - extraLarge: 1024, -}; + +enum TestSize { + ExtraLarge = 1024, + Large = 299, + Medium = 159, + Small = 99, +} describe('', () => { let spy: jest.Mock; @@ -223,7 +224,7 @@ describe('', () => { describe('overlayText', () => { const overlayText = 'overlay text'; it('does not render the overlayText on small screens', () => { - setBoundingClientRect('small'); + setBoundingClientRect(TestSize.Small); const dropZone = mountWithAppProvider( , ); @@ -234,8 +235,8 @@ describe('', () => { expect(caption).toHaveLength(0); }); - it('renders a Caption containing the overlayText on medium screens', () => { - setBoundingClientRect('medium'); + it('renders a Caption containing the overlayText when the bounding container is a medium size', () => { + setBoundingClientRect(TestSize.Medium); const dropZone = mountWithAppProvider( , ); @@ -244,8 +245,8 @@ describe('', () => { expect(captionText.contains(overlayText)).toBe(true); }); - it('renders a Caption containing the overlayText on large screens', () => { - setBoundingClientRect('large'); + it('renders a Caption containing the overlayText when the bounding container is a large size', () => { + setBoundingClientRect(TestSize.Large); const dropZone = mountWithAppProvider( , ); @@ -254,8 +255,8 @@ describe('', () => { expect(captionText.contains(overlayText)).toBe(true); }); - it('renders a DisplayText containing the overlayText on extra-large screens', () => { - setBoundingClientRect('extraLarge'); + it('renders a DisplayText containing the overlayText when the bounding container is an extra large size', () => { + setBoundingClientRect(TestSize.ExtraLarge); const dropZone = mountWithAppProvider( , ); @@ -268,7 +269,7 @@ describe('', () => { describe('errorOverlayText ', () => { const errorOverlayText = "can't drop this"; it("doesn't render the overlayText on small screens", () => { - setBoundingClientRect('small'); + setBoundingClientRect(TestSize.Small); const dropZone = mountWithAppProvider( , ); @@ -279,8 +280,8 @@ describe('', () => { expect(caption).toHaveLength(0); }); - it('renders a Caption containing the overlayText on medium screens', () => { - setBoundingClientRect('medium'); + it('renders a Caption containing the overlayText when the bounding container is a medium size', () => { + setBoundingClientRect(TestSize.Medium); const dropZone = mountWithAppProvider( , ); @@ -289,8 +290,8 @@ describe('', () => { expect(captionText.contains(errorOverlayText)).toBe(true); }); - it('renders a Caption containing the overlayText on large screens', () => { - setBoundingClientRect('large'); + it('renders a Caption containing the overlayText when the bounding container is a large size', () => { + setBoundingClientRect(TestSize.Large); const dropZone = mountWithAppProvider( , ); @@ -299,8 +300,8 @@ describe('', () => { expect(captionText.contains(errorOverlayText)).toBe(true); }); - it('renders a DisplayText containing the overlayText on extra-large screens', () => { - setBoundingClientRect('extraLarge'); + it('renders a DisplayText containing the overlayText when the bounding container is an extra large size', () => { + setBoundingClientRect(TestSize.ExtraLarge); const dropZone = mountWithAppProvider( , ); @@ -338,18 +339,20 @@ function createEvent(name: string, files: any) { return evt; } -function setBoundingClientRect(size: keyof typeof widths) { +function setBoundingClientRect(size: TestSize) { jest .spyOn(Element.prototype, 'getBoundingClientRect') .mockImplementation(() => { - return { - width: widths[size], - height: 100, + const rect: ClientRect = { + width: size, + height: size, top: 0, left: 0, bottom: 0, right: 0, }; + + return rect; }); } @@ -370,7 +373,7 @@ function fireEvent({ const event = createEvent(eventType, testFiles); element .find('div') - .at(3) + .at(0) .getDOMNode() .dispatchEvent(event); if (eventType === 'dragenter') { diff --git a/src/components/DropZone/types.ts b/src/components/DropZone/types.ts index d5f0baab7ab..eabe5134790 100755 --- a/src/components/DropZone/types.ts +++ b/src/components/DropZone/types.ts @@ -1,6 +1,13 @@ export type DropZoneEvent = DragEvent | React.ChangeEvent; +export enum Size { + ExtraLarge = 500, + Large = 300, + Medium = 160, + Small = 100, +} export interface DropZoneContext { - size: string; + width: Size; + height: Size; type: string; } From a205e04bc5e05e83d7b67070bb79698ec925bc81 Mon Sep 17 00:00:00 2001 From: Daniel Leroux Date: Thu, 25 Jul 2019 16:02:20 -0400 Subject: [PATCH 2/2] adding a11y change --- .vscode/settings.json | 2 +- src/components/DropZone/DropZone.tsx | 55 ++++++++++++---------------- 2 files changed, 24 insertions(+), 33 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 2f4a8ab8005..2ada6fff7a7 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -16,7 +16,7 @@ "**/node_modules": true, ".{dev,shadowenv.d}": true, "build-intermediate/": true, - "build/": true, + "build/": false, "docs/": true, "esnext/": true, "index.*": true, diff --git a/src/components/DropZone/DropZone.tsx b/src/components/DropZone/DropZone.tsx index d392705d72a..de13ea279a7 100755 --- a/src/components/DropZone/DropZone.tsx +++ b/src/components/DropZone/DropZone.tsx @@ -244,7 +244,6 @@ class DropZone extends React.Component { disabled, type: 'file', multiple: allowMultiple, - name: id, ref: this.fileInputNode, onChange: this.handleDrop, autoComplete: 'off', @@ -314,39 +313,31 @@ class DropZone extends React.Component { ? label : intl.translate('Polaris.DropZone.FileUpload.label'); const labelHiddenValue = label ? labelHidden : true; - const dropZoneMarkup = ( -
-
- {dragOverlay} - {dragErrorOverlay} -
{children}
- - - -
-
- ); - - const labelledDropzoneMarkup = label ? ( - - {dropZoneMarkup} - - ) : ( - dropZoneMarkup - ); return ( - {labelledDropzoneMarkup} + + +
+ {dragOverlay} + {dragErrorOverlay} +
{children}
+ + + +
+ + ); }