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

WIP - Add MediaUploadCheck component #9298

Closed
wants to merge 6 commits into from
Closed

Conversation

talldan
Copy link
Contributor

@talldan talldan commented Aug 24, 2018

Work in progress - not quite ready for review yet.

Description

Introduces a MediaUploadCheck component, which can be used to wrap other components and prevent them from rendering if a user does not have adequate permissions (e.g. Contributor roles) to upload files/media. This is implemented in our MediaPlaceholder component and several media blocks (image, video, audio etc.)

Currently this is hard-coded so that users won't see any change. A second PR will be created in a moment adding the checks for the user's capabilities.

These changes were originally implemented in #4155, but have been split into a separate PR to make them easier to review.

Related to issue #3672

Massive thanks to @imath for all the hard work he's put in developing this.

How has this been tested?

  • Manually tested with hard-coded value set to true, affected blocks behave normally
  • Manually tested with hard-coded value set to false, user is unable to upload media
  • Added unit tests

Screenshots

Screenshot of how media blocks would appear to users without capabilities
screen shot 2018-08-24 at 2 45 07 pm

Types of changes

New feature (non-breaking change which adds functionality)

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.

talldan and others added 6 commits August 24, 2018 14:31
- Hardcode hasUploadPermissions value to true for now until implementation of
server-side and redux state has been finalised

Co-authored-by: imath <[email protected]>
MediaUploadCheck component will conditionally show/hide parts of the UI
related to file uploads dependent on whether the user has the capability
to upload files.

Co-authored-by: imath <[email protected]>
Use early return when no featuredImageId is set. Reduce boolean checks around
component rendering.

Co-authored-by: imath <[email protected]>
@talldan talldan added [Type] Enhancement A suggestion for improvement. [Status] In Progress Tracking issues with work in progress [Feature] Media Anything that impacts the experience of managing media labels Aug 24, 2018
@talldan talldan self-assigned this Aug 24, 2018
/**
* Internal dependencies
*/
import { name, settings } from '../';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems a shame to remove these tests. I spent a while trying to find a way to mock 'select' and the hasUploadPermissions selector, but couldn't find a suitable way. Any pointers would be appreciated if there is a way to do this.

But I also thought that perhaps it would be better to spend the effort introducing e2e tests that cover the initial state of these blocks?

@talldan talldan closed this Aug 24, 2018
@talldan talldan deleted the add/media-check-component branch August 24, 2018 07:25
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 [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant