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

Adjust media upload source for rich text #10339

Merged
merged 3 commits into from Oct 5, 2018

Conversation

Projects
None yet
3 participants
@iseulde
Member

iseulde commented Oct 4, 2018

Description

  1. Fixes media blocks to use rich-text source on upload.
  2. Fixes file block transforms.
  3. Updates some docs to use rich-text instead of children.

How has this been tested?

Screenshots

Types of changes

Checklist:

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

@iseulde iseulde requested review from jorgefilipecosta and WordPress/gutenberg-core Oct 4, 2018

@iseulde iseulde added this to the 4.0 milestone Oct 4, 2018

@@ -109,7 +109,7 @@ export const settings = {
transform: ( attributes ) => {
return createBlock( 'core/file', {
href: attributes.src,
fileName: attributes.caption && attributes.caption.join(),
fileName: attributes.caption,

This comment has been minimized.

@iseulde

iseulde Oct 4, 2018

Member

Note that this wouldn't even work for more complex rich-text values...

@iseulde

iseulde Oct 4, 2018

Member

Note that this wouldn't even work for more complex rich-text values...

@@ -143,7 +143,7 @@ export const settings = {
transform: ( attributes ) => {
return createBlock( 'core/audio', {
src: attributes.href,
caption: [ attributes.fileName ],
caption: attributes.fileName,

This comment has been minimized.

@youknowriad

youknowriad Oct 4, 2018

Contributor

So the caption is a rich text type and the fileName is a string right? Should we call create here or something?

@youknowriad

youknowriad Oct 4, 2018

Contributor

So the caption is a rich text type and the fileName is a string right? Should we call create here or something?

This comment has been minimized.

@iseulde

iseulde Oct 4, 2018

Member

No, they are both rich text values. It's strange that the file name is not a plain text input field, but that's not in the scope of this PR.

@iseulde

iseulde Oct 4, 2018

Member

No, they are both rich text values. It's strange that the file name is not a plain text input field, but that's not in the scope of this PR.

@youknowriad

LGTM 👍

@jorgefilipecosta

This comment has been minimized.

Show comment
Hide comment
@jorgefilipecosta

jorgefilipecosta Oct 4, 2018

Member

As soon as an image is uploaded in the image/gallery placeholder, using the upload button we get a crash. I think we may need to apply create function from the RichText package when setting the attributes after the upload like we did in for the FileBlock.

Member

jorgefilipecosta commented Oct 4, 2018

As soon as an image is uploaded in the image/gallery placeholder, using the upload button we get a crash. I think we may need to apply create function from the RichText package when setting the attributes after the upload like we did in for the FileBlock.

@iseulde

This comment has been minimized.

Show comment
Hide comment
@iseulde

iseulde Oct 5, 2018

Member

@jorgefilipecosta I updated and rebased the PR with your latest changes.

Member

iseulde commented Oct 5, 2018

@jorgefilipecosta I updated and rebased the PR with your latest changes.

@jorgefilipecosta

Thank you for iterating on this changes @iseulde. Everything looks good in my tests now 👍

@iseulde

This comment has been minimized.

Show comment
Hide comment
@iseulde
Member

iseulde commented Oct 5, 2018

Thanks @jorgefilipecosta! :)

@iseulde iseulde merged commit 3057486 into master Oct 5, 2018

2 checks passed

codecov/project 49.32% (+0.01%) compared to dcd5cd1
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@iseulde iseulde deleted the fix/image-rich-text-source branch Oct 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment