Skip to content

Conversation

dleroux
Copy link
Contributor

@dleroux dleroux commented Nov 23, 2018

WHY are these changes introduced?

Because we render different components to output overlayText and errorOverlayText depending on screen size, the errorOverlayText rendered the wrong text.

WHAT is this pull request doing?

Ensures the right text is rendered and tests are added to prevent this in the future.

How to 🎩

Copy-paste this code in playground/Playground.tsx:
import * as React from 'react';
import {DropZone, Stack, Thumbnail, Caption} from '@shopify/polaris';

interface State {
  dirty: boolean;
  field: string;
  saving: boolean;
}

export default class DropZoneExample extends React.Component {
  state = {
    files: [],
  };

  render() {
    const {files} = this.state;
    const validImageTypes = ['image/gif', 'image/jpeg', 'image/png'];

    const fileUpload = !files.length && <DropZone.FileUpload />;
    const uploadedFiles = files.length > 0 && (
      <Stack vertical>
        {files.map((file, index) => (
          <Stack alignment="center" key={index}>
            <Thumbnail
              size="small"
              alt={file.name}
              source={
                validImageTypes.indexOf(file.type) > 0
                  ? window.URL.createObjectURL(file)
                  : 'https://cdn.shopify.com/s/files/1/0757/9955/files/New_Post.png?12678548500147524304'
              }
            />
            <div>
              {file.name} <Caption>{file.size} bytes</Caption>
            </div>
          </Stack>
        ))}
      </Stack>
    );

    return (
      <DropZone
        accept="image/*"
        type="image"
        onDrop={(files, acceptedFiles, rejectedFiles) => {
          this.setState({files: [...this.state.files, ...acceptedFiles]});
        }}
        errorOverlayText="Custom errorOverlayText"
      >
        {uploadedFiles}
        {fileUpload}
      </DropZone>
    );
  }
}

🎩 checklist

@BPScott BPScott temporarily deployed to polaris-react-pr-671 November 23, 2018 01:15 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-671 November 23, 2018 13:11 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-671 November 23, 2018 13:15 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-671 November 23, 2018 13:38 Inactive
@dleroux dleroux changed the title [WIP] [Drop Zone] Fix errorOverlayText prop [Drop Zone] Fix errorOverlayText prop on small screens Nov 23, 2018
@dleroux dleroux changed the title [Drop Zone] Fix errorOverlayText prop on small screens [WIP] Fix errorOverlayText prop on small screens Nov 23, 2018
@dleroux dleroux changed the title [WIP] Fix errorOverlayText prop on small screens Fix errorOverlayText prop on small screens Nov 23, 2018
@BPScott BPScott temporarily deployed to polaris-react-pr-671 November 23, 2018 18:16 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-671 November 26, 2018 20:58 Inactive
@danrosenthal
Copy link

@dleroux test failures look legitimate on this one

@BPScott BPScott temporarily deployed to polaris-react-pr-671 November 27, 2018 14:45 Inactive
@dleroux
Copy link
Contributor Author

dleroux commented Nov 27, 2018

Sorry about that, I hadn't noticed the failing CI. Fixed now @danrosenthal

Co-Authored-By: dleroux <dleroux@users.noreply.github.com>
Copy link
Member

@chloerice chloerice left a comment

Choose a reason for hiding this comment

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

Changes, tests, and 🎩 LGTM! 🚢

});

describe('errorOverlayText ', () => {
const errorOverlayText = "can't drop this";
Copy link
Member

Choose a reason for hiding this comment

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

😁

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should have put big pants or music notes here.

@dleroux dleroux merged commit 0218d3f into master Nov 30, 2018
@danrosenthal danrosenthal temporarily deployed to production December 4, 2018 19:16 Inactive
@kaelig kaelig deleted the fix-drop-zone branch December 10, 2018 23:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants