Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MediaUpload selection results in white screen when launched from a Modal. #12830

Open
hgezim opened this issue Dec 12, 2018 · 8 comments
Open
Labels
[Feature] Media Anything that impacts the experience of managing media [Type] Bug An existing feature does not function as intended

Comments

@hgezim
Copy link

hgezim commented Dec 12, 2018

Describe the bug
If you use Modal and have a MediaUpload component popup, when you select the image, the component goes blank (all white) and both modals are not recoverable.

To Reproduce
Steps to reproduce the behavior:

  1. Create a block with the following component in edit:
edit: withState ({
    isOpen: false,
    mediaId: null,
  }) (({isOpen, mediaId, setState}) => {
    return (
      <div>
        <Button isDefault onClick={() => setState ({isOpen: true})}>
          Open Modal
        </Button>
        {isOpen
          ? <Modal
              title="This is my modal"
              onRequestClose={() => setState ({isOpen: false})}
            >
              <MediaUploadCheck>
                <MediaUpload
                  onSelect={media => console.log ('selected ' + media.length)}
                  render={({open}) => (
                    <Button onClick={open}>
                      Open Media Library
                    </Button>
                  )}
                />
              </MediaUploadCheck>
            </Modal>
          : null}
      </div>
    );
  }),
  1. When you load the block, click Open Modal
  2. Click Open Media Library
  3. Select an image by clicking it.

Expected behavior
It should not show a white screen in the modal when the image is selected.

Screenshots
broken upload

Desktop (please complete the following information):

  • OS: Mac OS 10.14.1 (18B75)
  • Browser: Firefox
  • Version 63.0.3 (64-bit), 64.0 (64-bit) and Safari Version 12.0.1 (14606.2.104.1.1)

Additional context
Gutenberg version with WP 5.0

@swissspidy swissspidy added [Feature] Media Anything that impacts the experience of managing media Needs Testing Needs further testing to be confirmed. labels Dec 13, 2018
@pippaink
Copy link

pippaink commented Feb 21, 2019

I am experiencing the same issue. My case however is inside . If I move the MediaUpload code to outside the Dropdown it works. Inside Dropdown I get a blank screen on image select or upload. It seems whenever MediaUpload is used inside other components it breaks.

There are no javascript console errors. The HTML output is empty for the media modal.
<div class="media-modal-content"></div>

  • OS: Mac OS 10.14.1
  • Browser: Chrome Version 72.0.3626.109 (Official Build) (64-bit)

@hos-shams
Copy link

Same here when MediaUpload is inside Dropdown.

@davilera
Copy link
Contributor

Me too, I have the exact same problem as @shamsoft’s

@davilera
Copy link
Contributor

I think I know what's amiss (even though I have no idea on how to fix it). When I click on the MediaUpload, I'm actually clicking outside the Dropdown, which closes the Dropdown and, therefore, destroys the MediaUpload... or something like that.

@talldan talldan removed the Needs Testing Needs further testing to be confirmed. label Apr 2, 2020
@talldan
Copy link
Contributor

talldan commented Apr 2, 2020

I can confirm what @davilera mentions having noticed similar before. It's this bit of code that does that:

componentWillUnmount() {
this.frame.remove();
}

A way to stop it would be to stop the modal behind the uploader from closing.

It might be possible to check whether the event that triggers closing happened inside the media uploader and then avoid closing the modal if that's the case.

The callback passed to onRequestClose receives the event as an argument:
https://github.com/WordPress/gutenberg/blob/master/packages/components/src/modal/frame.js#L97

So hopefully it's possible to do something like this would work:

onRequestClose={ ( event ) => {
  if ( event.target.closest( '.media-frame' ) ) {
    return;
  }

  // else close the modal
} }

Though note that closest isn't supported in IE.

For dropdowns/popovers I'm not sure if there's something similar.

@talldan talldan added the [Type] Bug An existing feature does not function as intended label Apr 2, 2020
@manzoorwanijk
Copy link
Member

Facing the same issue, any update?

kienstra added a commit to studiopress/genesis-custom-blocks that referenced this issue Sep 6, 2021
@aurooba
Copy link
Member

aurooba commented Dec 19, 2021

Can confirm I'm seeing the same issue. Would love to see this resolved in core!

@talldan
Copy link
Contributor

talldan commented Jul 24, 2024

Revisiting this, I think the main issue is that the MediaUpload works the way it does:

<MediaUpload
  onSelect={media => console.log ('selected ' + media.length)}
  render={({open}) => (
    <Button onClick={open}>
      Open Media Library
    </Button>
  )}
/>

Having to render the opener button in a render callback is the problem. That forces you to render MediaUpload as a child of the Modal to make the button appear there, whereas it should instead be a sibling.

If it worked like a regular Modal component it'd be better:

edit: withState ({
  isOpen: false,
  isMediaLibraryOpen: false,
  mediaId: null,
}) (({isOpen, mediaId, setState}) => {
  return (
    <div>
      <Button isDefault onClick={() => setState ({isOpen: true})}>
        Open Modal
      </Button>
      {isOpen
        ? <Modal
            title="This is my modal"
            onRequestClose={() => setState ({isOpen: false})}
          >
            <MediaUploadCheck>
              <Button onClick={() => setState({isMediaLibraryOpen: true })}>
                Open Media Library
              </Button>
            </MediaUploadCheck>
          </Modal>
        : null}
      { isMediaLibraryOpen && 
        <MediaUpload
          onSelect={media => console.log ('selected ' + media.length)}
        />
      }
    </div>
  );
})

That said, I think anyone reading can implement the steps mentioned in this comment or in my previous comment to work around it, though on that other issue it was mentioned there might also be z-index problems.

I think there's a question over how much value there is in solving this one this given a media library rework is on the cards in the very near future - #55238. Though there may be a requirement to make MediaUpload work in a future compatible way that's more like a modal if it remains the way to open the new media library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Media Anything that impacts the experience of managing media [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

8 participants