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

Fix: Contributors can not use drag & drop #15054

Conversation

@jorgefilipecosta
Copy link
Member

jorgefilipecosta commented Apr 18, 2019

Currently, if the user has no permission to upload files, the user is not able to use block drag & drop.
This PR fixes the problem.

How has this been tested?

I added some paragraphs in Gutenberg playground.
I hovered with the mouse at the left side of the paragraph until the hand to grab the paragraph appeared. The reason why the block movers don't become visible on the playground is probably another bug that I will try to find the cause.
I changed the paragraph position by dropping it above the blue line in another position.

I tried to drag a file to the playground and verified it was not possible to do it and the blue line to drop never appeared -- in the playground the user is not able to upload files.

Copy link
Member

nosolosw left a comment

This will render the dropzone and will allow the user to drop an image from the desktop. It won't work, though, because the DropZoneProvider checks whether the DnD type is supported. We should take the DnD type into account in the DropZoneComponent's render method as well.

@@ -116,17 +116,20 @@ class BlockDropZone extends Component {
return null;
}
const isAppender = index === undefined;

const dropZone = (

This comment has been minimized.

Copy link
@nosolosw

nosolosw Apr 22, 2019

Member

Was thinking how to improve readability here to better communicate the difference. Some raw thoughts:

  1. Do not use the MediaUploadCheck component but use the hasUploadPermissions prop:
<DropZone
    className={ ... }
    onHTMLDrop={ this.onHTMLDrop }
    onDrop={ this.onDrop }
    onFilesDrop={ hasUploadPermissions ? this.onFilesDrop : null }
    />
  1. Extract and rename the dropzone components so we return:
return (
    <MediaUploadCheck
        fallback={ dropZoneWithoutFileDrop }>
        dropZoneWithFileDrop
    </MediaUploadCheck>
);

This comment has been minimized.

Copy link
@jorgefilipecosta

jorgefilipecosta May 28, 2019

Author Member

Hi @nosolosw, I updated the code to follow your first suggestion.

@jorgefilipecosta jorgefilipecosta force-pushed the fix/implosible-to-drag-drop-blocks-if-the-user-can-not-upload-files branch from c5821e1 to ee3cc38 May 28, 2019
@jorgefilipecosta

This comment has been minimized.

Copy link
Member Author

jorgefilipecosta commented May 28, 2019

Hi @nosolosw thank you for the review, I applied a code simplification based on your suggestions.

This will render the dropzone and will allow the user to drop an image from the desktop. It won't work, though, because the DropZoneProvider checks whether the DnD type is supported. We should take the DnD type into account in the DropZoneComponent's render method as well.

In my tests, I can drag & drop blocks and files in the normal editor, and on the widgets editor (/admin.php?page=gutenberg-widgets) I can drag blocks but not files because uploads are not supported yet. This seems to work exectly as expected I'm probably missing a usage, what is failing in your tests/what are the steps to reproduce the problem?

@nosolosw

This comment has been minimized.

Copy link
Member

nosolosw commented May 29, 2019

These are my testing steps:

  • Create a user with contributor role (it doesn't have upload permissions).
  • Open the editor.
  • Grab an image from desktop and drop it into the editor

In master the image won't be dropped in the editor.

In this branch, it will, although unsuccessfully (an user without upload permissions shouldn't be able to drag anything from the desktop):

contributor-can-upload-2019-05-29-11-34

@jorgefilipecosta jorgefilipecosta force-pushed the fix/implosible-to-drag-drop-blocks-if-the-user-can-not-upload-files branch from ee3cc38 to 56fc5b7 May 29, 2019
@jorgefilipecosta

This comment has been minimized.

Copy link
Member Author

jorgefilipecosta commented May 29, 2019

Thank you for providing clear steps to reproduce the issue you found @nosolosw 👍 The problem was fixed.

@nosolosw

This comment has been minimized.

Copy link
Member

nosolosw commented May 30, 2019

I still can upload files with the steps above (although the visual indicator isn't rendered).

@jorgefilipecosta jorgefilipecosta force-pushed the fix/implosible-to-drag-drop-blocks-if-the-user-can-not-upload-files branch from 56fc5b7 to 9a6ff94 Jun 4, 2019
@jorgefilipecosta

This comment has been minimized.

Copy link
Member Author

jorgefilipecosta commented Jun 4, 2019

I still can upload files with the steps above (although the visual indicator isn't rendered).

Hi @nosolosw, I'm not being able to reproduce the problem. For contributor roles, the blue bar does not appear and the upload does not happen if I drag a file over the position the blue bar would appear. Maybe there was a caching issue? Would it be possible to do another try? Thank you in advance!
Edited: It may also be browser related, could you please share what browser was used in the tests.

@nosolosw nosolosw self-requested a review Jul 17, 2019
@jorgefilipecosta jorgefilipecosta force-pushed the fix/implosible-to-drag-drop-blocks-if-the-user-can-not-upload-files branch from 9a6ff94 to 8452a32 Aug 6, 2019
@jorgefilipecosta jorgefilipecosta force-pushed the fix/implosible-to-drag-drop-blocks-if-the-user-can-not-upload-files branch from 8452a32 to bce23fc Oct 14, 2019
@jorgefilipecosta jorgefilipecosta dismissed nosolosw’s stale review Oct 14, 2019

reviews were addressed

@jorgefilipecosta

This comment has been minimized.

Copy link
Member Author

jorgefilipecosta commented Oct 14, 2019

This PR was rebased and retested and it seems to still be valid.
Most of the PR can easily be live tested using the playground available https://deploy-preview-15054--gutenberg-playground.netlify.com/.

@nosolosw

This comment has been minimized.

Copy link
Member

nosolosw commented Oct 31, 2019

I gave it a spin today at this. This is what I've done:

  • Created a new user with contributor role.
  • Opened the demo Gutenberg content.
  • Tested that the contributor could drag and drop things around (they see the blue bar indicating the target and so on). This is great and expected!
  • Tested dragging files from the desktop: although the blue bar indicating the target isn't shown, in some situations I was able to drop the image from desktop, resulting in an error. See below.

GIF (note how the first time I dropped the image nothing happened, but the second time an invalid block was created):

Peek 2019-10-31 12-00

The code for the invalid block:

<!-- wp:image -->
<figure class="wp-block-image">
    <img
        src="blob:http://localhost:8889/6a9516cb-fc73-4283-9b10-3ac959c110bc"
        alt=""/>
</figure>
<!-- /wp:image -->
@jorgefilipecosta jorgefilipecosta force-pushed the fix/implosible-to-drag-drop-blocks-if-the-user-can-not-upload-files branch from bce23fc to 0e5dc2d Nov 13, 2019
@jorgefilipecosta

This comment has been minimized.

Copy link
Member Author

jorgefilipecosta commented Nov 13, 2019

Hi @nosolosw thank you for the review! Unfortunately, I was never able to replicate the problem. But from the issue description and gif, it seems for some reason onFilesDrop is being called. I added a check for upload permissions inside onFileDrop.

@jorgefilipecosta jorgefilipecosta changed the title Fix: Impossible to drag & drop blocks if the user has no upload files permission Fix: Contributors can not use drag & drop Nov 14, 2019
Copy link
Member

nosolosw left a comment

Now DnD works for contributors as well!

@jorgefilipecosta jorgefilipecosta merged commit 95c3f5e into master Nov 19, 2019
2 checks passed
2 checks passed
pull-request-automation
Details
Travis CI - Pull Request Build Passed
Details
@jorgefilipecosta jorgefilipecosta deleted the fix/implosible-to-drag-drop-blocks-if-the-user-can-not-upload-files branch Nov 19, 2019
mkevins added a commit that referenced this pull request Nov 19, 2019
@youknowriad youknowriad added this to the Gutenberg 7.0 milestone Nov 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.