[WIP][DropZone] Allow fixed height on DropZone

[elileto]

WHY are these changes introduced?

Resolves #742

UPDATE:
When a height is specified the consumer will need to also adjust the horizontal alignment of anything they put within the DropZone (including a FileUpload component.)

The DropZone determines its size (height and width) based on width only. This makes certain valid use cases, where the default DropZone height isn't appropriate, impossible.

Important Notes - Needs Discussion

This PR was originally merged #908 however there was an issue with putting a flex on Dropzone_Container which caused DropZone.FileUpload to centre horizontally however this didn't take into account the fact that DropZone could have other children.

What is up for discussion is that when a fixed height is given to the DropZone should it be up to the consumer to style DropZone's children or should we have two conditions, one that takes into account if FileUpload is the only child (therefore center it ) and the other being do not center FileUpload when there are multiple children.

There is also the fix Dan R. proposes here which adds a prop that center-aligns children and defaults to true.

WHAT is this pull request doing?

This pull request allows the consumer to set a fixed height on a wrapping parent element so that the DropZone takes its parent's height if one is provided.

Current Behaviour: (wrapped with a div given a height of 50px)

New Behaviour: (wrapped with a div given a height of 50px)

(Note: there are in between size variations (ie. small width, medium height etc.) The FileUpload will always render the smaller measurement of width or height.

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Example of All Dropzone Possibilities

import * as React from 'react';
import {
  Page,
  DropZone,
  Stack,
  Thumbnail,
  Caption,
  Heading,
  Subheading,
} from '../src';

interface State {
  files: object;
}

export default class Playground extends React.Component<{}, State> {
  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 (
      <Page title="Dropzones">
        <Heading> Small Height : Small Width </Heading>
        <div style={{width: 50, height: 50}}>
          <DropZone
            onDrop={(files, acceptedFiles, rejectedFiles) => {
              this.setState({
                files: [...files, ...acceptedFiles],
              });
            }}
          >
            {uploadedFiles}
            {fileUpload}
          </DropZone>
        </div>
        <Heading> Medium Height : Medium Width </Heading>
        <div style={{width: 150, height: 150}}>
          <DropZone
            onDrop={(files, acceptedFiles, rejectedFiles) => {
              this.setState({
                files: [...files, ...acceptedFiles],
              });
            }}
          >
            {uploadedFiles}
            {fileUpload}
          </DropZone>
        </div>
        <Heading> Large Height : Large Width </Heading>
        <div style={{width: 250, height: 250}}>
          <DropZone
            onDrop={(files, acceptedFiles, rejectedFiles) => {
              this.setState({
                files: [...files, ...acceptedFiles],
              });
            }}
          >
            {uploadedFiles}
            {fileUpload}
          </DropZone>
        </div>
        <Heading> ExtraLarge Height : ExtraLarge Width </Heading>
        <div style={{width: 500, height: 500}}>
          <DropZone
            onDrop={(files, acceptedFiles, rejectedFiles) => {
              this.setState({
                files: [...files, ...acceptedFiles],
              });
            }}
          >
            {uploadedFiles}
            {fileUpload}
          </DropZone>
        </div>
        <Heading> Default Height : Default Width </Heading>
        <Subheading>mobile and full width</Subheading>
        <DropZone
          onDrop={(files, acceptedFiles, rejectedFiles) => {
            this.setState({
              files: [...files, ...acceptedFiles],
            });
          }}
        >
          {uploadedFiles}
          {fileUpload}
        </DropZone>
        <Heading> ExtraLarge Height : Small Width </Heading>
        <div style={{width: 50, height: 500}}>
          <DropZone
            onDrop={(files, acceptedFiles, rejectedFiles) => {
              this.setState({
                files: [...files, ...acceptedFiles],
              });
            }}
          >
            {uploadedFiles}
            {fileUpload}
          </DropZone>
        </div>
        <Heading> Small Height : ExtraLarge Width </Heading>
        <div style={{width: 500, height: 50}}>
          <DropZone
            onDrop={(files, acceptedFiles, rejectedFiles) => {
              this.setState({
                files: [...files, ...acceptedFiles],
              });
            }}
          >
            {uploadedFiles}
            {fileUpload}
          </DropZone>
        </div>
      </Page>
    );
  }
}

🎩 checklist

• Tested on mobile
• Tested on multiple browsers
• Tested for accessibility

[elileto]

@dleroux @ry5n @danrosenthal I'm tagging you all to discuss the options that this PR produces and to also make sure I didn't miss anything in my notes above about the options. Happy to also keep looking into solutions for this in the meantime! Thank you.

[mirualves]

Hi @elizabethletourneau! @StephPoce talked about this issue today. I just want to let you know that I'll take a look at that from a design perspective. Let me organize my next few days and all the catch up after vacation.

[mirualves]

So I took a look at the related issues, and @elizabethletourneau helped me to understand a bit more of the problem (thank you, Liz! <3)

Taking in consideration the fact that Boolean props should often be false by default (unless there’s a legitimate exception) and, paraphrasing @danrosenthal on issue #1127, “its not typically desirable to have parents lay out their children” I believe is safe to accept that it should be up to the consumer to style drop zone’s children if they want to customize the height of the component.

I thought that maybe there’s an opportunity here to add some guidelines to the style guide in how to do centralize it horizontally once you customize the height of the drop zone.

Unrelated question: I was playing with the props and tried the Disabled one. Specifically, when we have the option of having a “Add file” button, should the button appear on the disabled state if the disabled prop is set to true?

[AndrewMusgrave]

Looking really good 😄 Left a few small comments but if we're 👌 from a design perspective this should be almost ready to ship since Dan, Solona and myself all gave 👍 on #908 and these pr's are almost identical

[AndrewMusgrave]

Suggested change

	Constrain DropZone height based on inherited wrapper height (#1138)[https://github.com/Shopify/polaris-react/pull/1138]
	- Constrain DropZone height based on inherited wrapper height ([#1138](https://github.com/Shopify/polaris-react/pull/1138))

[AndrewMusgrave]

Should we move this inside the if statement since it's not used outside?

[chloerice]

💯

[AndrewMusgrave]

Since the default is ExtraLarge we can 🔥 the else statement

[elileto]

🔥 🔥 🔥

[AndrewMusgrave]

🎩 ✅ cc/ @mirualves for design 👀

[AndrewMusgrave]

Suggested change

	Constrain DropZone height based on inherited wrapper height ([#1138](https://github.com/Shopify/polaris-react/pull/1138))
	- Constrain DropZone height based on inherited wrapper height ([#1138](https://github.com/Shopify/polaris-react/pull/1138))

[chloerice]

Hey @AndrewMusgrave, should I rebase and merge this, or are we still looking for design input first?

[mirualves]

I think we're fine on the design, @chloerice !

[dleroux]

@chloerice I just rebased and tests are failing. Any idea what changed? I can dig into it but just wondering if you knew before spending too much time on this.

[chloerice]

@chloerice I just rebased and tests are failing. Any idea what changed? I can dig into it but just wondering if you knew before spending too much time on this.

Tests started failing after my rebase as well. I haven't been successful in finding what's going on since the rebase, was digging into the higher priority issue. I'll be looking at this again Wednesday morning, but feel free to poke at it in the meantime!

[dleroux]

I've rebased this and fixed tests.

@mirualves you're good with the Percy changes?

@dpersing note the a11y issues. What's the recommendation here?

[mirualves]

@dleroux Let's investigate why it's affecting the spacing between the illustration, button and hint text. Ideally, it should have the same spacing as before.

[dleroux]

I checked the spacing @mirualves and the Stack is behaving correctly. The new dimensions are what is causing it so I think it's fine.

[dleroux]

Now that label is always required something is broken with the height.

[dpersing]

note the a11y issues. What's the recommendation here?

@dleroux Are you referencing automated tests (which are passing now, it looks), or the a11y issues logged against the component?

[dleroux]

Yes @dpersing. Sorry I figured it out. The accessibility fix (always having a label) is what breaks the visual layout right now.

[danrosenthal]

This PR has been open for about 6 months, I'm thinking we ought to close it until someone can commit to this change and see it through.

[dleroux]

You see an issue with leaving it open? I'm ok with closing this but I just worry it might be completely forgotten.

[dpersing]

@danrosenthal @daniedleroux Closing the PR should be okay if the associated issue remains open?