Skip to content

Blocks: Media upload docs improvements#5846

Merged
gziolo merged 1 commit intomasterfrom
update/media-upload-improvements
Apr 4, 2018
Merged

Blocks: Media upload docs improvements#5846
gziolo merged 1 commit intomasterfrom
update/media-upload-improvements

Conversation

@gziolo
Copy link
Copy Markdown
Member

@gziolo gziolo commented Mar 28, 2018

Description

Follow-up for #5211:

At the moment MediaUpload component depends on wp.media, which makes also blocks module dependent on media module. This PR tries to move this dependency to the edit-post in Gutenberg and give more flexibility on how images are inserted into the post.

This PR adds missing documentation which explains how MediaUpload component can be overridden and explains why it is structured this way.

When working on the docs I came up to the conclusion that MediaUpload can be now moved to the components module. I added deprecation logic to make sure that there is proper feedback provided for the developers that decided to use this component.

How Has This Been Tested?

Manually tested to confirm that there are no regressions. It should be still possible

Run npm test to make sure all tests still pass.

@gziolo gziolo added [Type] Developer Documentation Documentation for developers [Feature] Extensibility The ability to extend blocks or the editing experience [Feature] Media Anything that impacts the experience of managing media [Feature] UI Components Impacts or related to the UI component system labels Mar 28, 2018
@gziolo gziolo self-assigned this Mar 28, 2018
@gziolo gziolo requested review from mcsf and youknowriad March 28, 2018 15:43
youknowriad
youknowriad reviewed Mar 28, 2018
View reviewed changes
components/media-upload/index.js Outdated
Copy link
Copy Markdown
Contributor

@youknowriad youknowriad Mar 28, 2018

Choose a reason for hiding this comment

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

I'm not sure we want to move components relying on filters to the components folder. I feel like the components folder is for generic React components while adding filters adds an unwanted dependency the wp hooks.

I guess it's probably not the first component doing this, but I wanted to share this concern of mine.

Copy link
Copy Markdown
Member Author

@gziolo gziolo Mar 28, 2018

Choose a reason for hiding this comment

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

@aduth @mtias @mcsf, what is your take on this? I'm fine with leaving it where it was if you disagree with the move. All the docs I added are still valuable :)

Copy link
Copy Markdown
Member

@mtias mtias Apr 2, 2018

Choose a reason for hiding this comment

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

In general, I see this is a block tool, so placing it in the blocks folder makes sense. Not sure how that will evolve, but I expect most block API UI items to be there.

Copy link
Copy Markdown
Member Author

@gziolo gziolo Apr 3, 2018

Choose a reason for hiding this comment

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

Thanks for leaving your feedback 👍

@gziolo gziolo force-pushed the update/media-upload-improvements branch from 9936b60 to d072b90 Compare March 28, 2018 16:12
@gziolo gziolo requested review from aduth and mtias March 29, 2018 10:31
@gziolo gziolo added this to the 2.6 milestone Mar 29, 2018
@gziolo gziolo mentioned this pull request Apr 3, 2018
Merged
3 tasks
@gziolo gziolo changed the title Components: Media upload refactor and docs improvements Blocks: Media upload docs improvements Apr 3, 2018
@gziolo gziolo force-pushed the update/media-upload-improvements branch from d072b90 to 35df645 Compare April 3, 2018 09:11
@gziolo
Copy link
Copy Markdown
Member Author

gziolo commented Apr 3, 2018

Updated PR to include only docs update.

@gziolo gziolo removed the [Feature] UI Components Impacts or related to the UI component system label Apr 3, 2018
@gziolo gziolo force-pushed the update/media-upload-improvements branch from 35df645 to f0a828b Compare April 3, 2018 09:13
@gziolo gziolo force-pushed the update/media-upload-improvements branch from f0a828b to 96b1817 Compare April 3, 2018 09:14
@gziolo gziolo requested a review from brandonpayton April 3, 2018 09:16
youknowriad
youknowriad approved these changes Apr 4, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@gziolo gziolo merged commit 69c69cf into master Apr 4, 2018
@gziolo gziolo deleted the update/media-upload-improvements branch April 4, 2018 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@youknowriad youknowriad youknowriad approved these changes

@mcsf mcsf Awaiting requested review from mcsf

@aduth aduth Awaiting requested review from aduth

@mtias mtias Awaiting requested review from mtias

@brandonpayton brandonpayton Awaiting requested review from brandonpayton

Assignees

@gziolo gziolo

Labels

[Feature] Extensibility The ability to extend blocks or the editing experience [Feature] Media Anything that impacts the experience of managing media [Type] Developer Documentation Documentation for developers

Projects

None yet

Milestone

2.6

Development

Successfully merging this pull request may close these issues.

3 participants

@gziolo @youknowriad @mtias