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

Add media button to classic block #10306

Merged
merged 1 commit into from Oct 3, 2018

Conversation

Projects
None yet
5 participants
@iseulde
Member

iseulde commented Oct 3, 2018

Description

Fixes #7762.

How has this been tested?

Insert a classic block, observe media button on the right side of the toolbar. Try to insert images.

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 a review from WordPress/gutenberg-core Oct 3, 2018

@tofumatt tofumatt added this to the 4.0 milestone Oct 3, 2018

@tofumatt tofumatt self-assigned this Oct 3, 2018

@tofumatt

tofumatt approved these changes Oct 3, 2018 edited

It looks like inserting just an image into the Classic Block causes the autosave to fail:

2018-10-03 12 27 37

The API thinks the post is empty. That's quite strange, but I'm thinking it's an unrelated bug. Is that known? 🤷‍♂️

Anyway, the code here seems good and it works otherwise. As long as that's a known bug already (or is a separate bug) this is fine.

@iseulde

This comment has been minimized.

Show comment
Hide comment
@iseulde

iseulde Oct 3, 2018

Member

I’ll have a look separately at that.

Member

iseulde commented Oct 3, 2018

I’ll have a look separately at that.

@iseulde

This comment has been minimized.

Show comment
Hide comment
@iseulde

iseulde Oct 3, 2018

Member

Thanks Matt!

Member

iseulde commented Oct 3, 2018

Thanks Matt!

@iseulde iseulde merged commit 5e13281 into master Oct 3, 2018

2 checks passed

codecov/project 49.42% (-0.01%) compared to c178f1d
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@afercia

This comment has been minimized.

Show comment
Hide comment
@afercia

afercia Oct 3, 2018

Contributor

@tofumatt for me the autosave fail happens also with only text. Just leave the post title empty, and there will be an error 500. I guess it should go in a separate issue.

screen shot 2018-10-03 at 19 52 10

Contributor

afercia commented Oct 3, 2018

@tofumatt for me the autosave fail happens also with only text. Just leave the post title empty, and there will be an error 500. I guess it should go in a separate issue.

screen shot 2018-10-03 at 19 52 10

@tofumatt

This comment has been minimized.

Show comment
Hide comment
@tofumatt

tofumatt Oct 3, 2018

Member
Member

tofumatt commented Oct 3, 2018

@youknowriad youknowriad deleted the add/classic-media-button branch Oct 3, 2018

@maddisondesigns

This comment has been minimized.

Show comment
Hide comment
@maddisondesigns

maddisondesigns Oct 4, 2018

If you're adding a media button to the Classic Block toolbar, can you please add the same button to the Paragraph Block. It's going to be incredibly confusing for users if you have to click a toolbar button to insert an image in one block, but in another block you have to click the Block Inserter and insert an Inline Block. Not only is it going to be incredibly confusing, but it's inconsistent. It's performing the exact same function (inserting an image), in two completely different ways, depending what block you're on.

maddisondesigns commented Oct 4, 2018

If you're adding a media button to the Classic Block toolbar, can you please add the same button to the Paragraph Block. It's going to be incredibly confusing for users if you have to click a toolbar button to insert an image in one block, but in another block you have to click the Block Inserter and insert an Inline Block. Not only is it going to be incredibly confusing, but it's inconsistent. It's performing the exact same function (inserting an image), in two completely different ways, depending what block you're on.

@tofumatt

This comment has been minimized.

Show comment
Hide comment
@tofumatt

tofumatt Oct 4, 2018

Member

The inline image inside a paragraph is, in my mind, a bit of a special case because of how online publications frequently have inline images in stories, etc. The same is less true for video, though not totally uncommon. That said, I think that's a great third-party block idea.

I don't see adding further inline media elements to the paragraph block as a good idea right now. I don't see our current implementation as inconsistent; the Classic Block is an entirely different UX than our new paragraph block and the new Paragraph block doesn't aim to replicate all of the Classic Block's feature set.

That said: that's a feature request that would be best filed as a new issue and not a PR comment 😄. In the future please feel free to file follow-up issues instead of commenting on PRs, so we can better triage things and keep discussions more focused. Thanks! 🙂

Member

tofumatt commented Oct 4, 2018

The inline image inside a paragraph is, in my mind, a bit of a special case because of how online publications frequently have inline images in stories, etc. The same is less true for video, though not totally uncommon. That said, I think that's a great third-party block idea.

I don't see adding further inline media elements to the paragraph block as a good idea right now. I don't see our current implementation as inconsistent; the Classic Block is an entirely different UX than our new paragraph block and the new Paragraph block doesn't aim to replicate all of the Classic Block's feature set.

That said: that's a feature request that would be best filed as a new issue and not a PR comment 😄. In the future please feel free to file follow-up issues instead of commenting on PRs, so we can better triage things and keep discussions more focused. Thanks! 🙂

@maddisondesigns

This comment has been minimized.

Show comment
Hide comment
@maddisondesigns

maddisondesigns Oct 4, 2018

@tofumatt I raised this exact concern over 2 months ago on the original issue that I raised in regards to not being able to add inline images in the classic block - #7762 (comment)

maddisondesigns commented Oct 4, 2018

@tofumatt I raised this exact concern over 2 months ago on the original issue that I raised in regards to not being able to add inline images in the classic block - #7762 (comment)

@iseulde

This comment has been minimized.

Show comment
Hide comment
@iseulde

iseulde Oct 4, 2018

Member

The classic block should not be compared to any other block. It's there for backward compatibility. Of course these blocks will have entirely different UI. Also you can insert images in the paragraph block right now. Open this inserter while focus is in the paragraph block, then open the "inline blocks" panel, and click "inline image".

Member

iseulde commented Oct 4, 2018

The classic block should not be compared to any other block. It's there for backward compatibility. Of course these blocks will have entirely different UI. Also you can insert images in the paragraph block right now. Open this inserter while focus is in the paragraph block, then open the "inline blocks" panel, and click "inline image".

@maddisondesigns

This comment has been minimized.

Show comment
Hide comment
@maddisondesigns

maddisondesigns Oct 4, 2018

@iseulde I realise that you can insert an image in the Paragraph block, which is why I'm saying that it makes sense to have the same workflow on both blocks. As far as any user is concerned, the Paragraph Block and the Classic Block are just two separate blocks. They both reside in the Block Inserter and there's no obvious indicator that the Classic Block is some 'special' kind of block that should be treated any differently to any other block. Even the description for the Classic Block (in the sidebar) indicates that it's just another block. It makes no sense to have a different 'Insert image' UX for both of them, and the media button that you've added to the Classic Block toolbar is a much more user friendly approach to adding inline images, that having to insert a separate Inline Image block inside of the Paragraph Block.

maddisondesigns commented Oct 4, 2018

@iseulde I realise that you can insert an image in the Paragraph block, which is why I'm saying that it makes sense to have the same workflow on both blocks. As far as any user is concerned, the Paragraph Block and the Classic Block are just two separate blocks. They both reside in the Block Inserter and there's no obvious indicator that the Classic Block is some 'special' kind of block that should be treated any differently to any other block. Even the description for the Classic Block (in the sidebar) indicates that it's just another block. It makes no sense to have a different 'Insert image' UX for both of them, and the media button that you've added to the Classic Block toolbar is a much more user friendly approach to adding inline images, that having to insert a separate Inline Image block inside of the Paragraph Block.

@iseulde

This comment has been minimized.

Show comment
Hide comment
@iseulde

iseulde Oct 4, 2018

Member

The fact that the UI is the same as the previous editor should make it clear that the block is something different. The UI will also not disappear once it's there. If it's not clear enough that the classic block is something special, we might want to communicate that better. It's really part of the Gutenberg experience and we would prefer that people use other blocks instead.

That said, media in the classic block is very different than media in Gutenberg. It's not possible to add any media (image, gallery, audio...) Gutenberg blocks inside the Classic block, so it makes more sense for the button to be more prominent. Adding inline images inside the normal Gutenberg blocks should be much rarer while we have dedicated blocks that you can insert at the block level. Inline images are really just inserted in the line of text and meant to be small.

Member

iseulde commented Oct 4, 2018

The fact that the UI is the same as the previous editor should make it clear that the block is something different. The UI will also not disappear once it's there. If it's not clear enough that the classic block is something special, we might want to communicate that better. It's really part of the Gutenberg experience and we would prefer that people use other blocks instead.

That said, media in the classic block is very different than media in Gutenberg. It's not possible to add any media (image, gallery, audio...) Gutenberg blocks inside the Classic block, so it makes more sense for the button to be more prominent. Adding inline images inside the normal Gutenberg blocks should be much rarer while we have dedicated blocks that you can insert at the block level. Inline images are really just inserted in the line of text and meant to be small.

@maddisondesigns

This comment has been minimized.

Show comment
Hide comment
@maddisondesigns

maddisondesigns Oct 5, 2018

The fact that the UI is the same as the previous editor should make it clear that the block is something different.

That is a huge assumption, and I am confident when I say that your average user wont make that same assumption. You're looking at it from a developers perspective, not as an end user. All they will see is that it's another block. They wont care less that it looks slightly different. To them, it's just another block that they can use. It literally says it right in the name, Classic Block.

It's really part of the Gutenberg experience and we would prefer that people use other blocks instead.

You may prefer this, but it's not going to happen. You can't make assumptions on how people will use this, and likewise, you can't force people not to use the Classic Block if they want to.

That said, media in the classic block is very different than media in Gutenberg.

How is the media different? When you insert an Inline Image Block, all it does is that it allows you to select an image and then it inserts the appropriate <img> tag into the Paragraph Block. How is this different to what you're going to be doing for the Classic Block? You're simply going to insert an image in exactly the same way. They're not different. It's exactly the same thing. The only difference is how you go about inserting the image. The end results though, is identical. This is what a classic block looks like after inserting and Inline Image block...

screenshot_872

It's not possible to add any media (image, gallery, audio...) Gutenberg blocks inside the Classic block, so it makes more sense for the button to be more prominent.

Nowhere have I suggested that it should be possible to insert Gutenberg Blocks inside a classic block. All I've suggested is that you add a media button to the Paragraph block so that people can insert inline images in exactly the same way as they'll be inserting them into the Classic Block, rather than making them insert an Inline Image Block. Having a media button on the toolbar is going to be a significantly easier experience for the end user, than having to Insert and Inline Image block, and it keeps the ui more consistent.

Adding inline images inside the normal Gutenberg blocks should be much rarer while we have dedicated blocks that you can insert at the block level.

This is completely inaccurate. Anyone who writes documentation will use inline images. If you're describing the use of an icon, for example, it helps to have an small image of the icon inline so your readers know exactly which icon you're referring to.

Inline images are really just inserted in the line of text and meant to be small.

I fully realise that but that really doesn't have anything to do with anything that we've been discussing.

Based on your comments above, it's clear that you have no intention of changing it at this point, but personally, I think that's a mistake and it's only making the ui harder and more complicated for the end user.

maddisondesigns commented Oct 5, 2018

The fact that the UI is the same as the previous editor should make it clear that the block is something different.

That is a huge assumption, and I am confident when I say that your average user wont make that same assumption. You're looking at it from a developers perspective, not as an end user. All they will see is that it's another block. They wont care less that it looks slightly different. To them, it's just another block that they can use. It literally says it right in the name, Classic Block.

It's really part of the Gutenberg experience and we would prefer that people use other blocks instead.

You may prefer this, but it's not going to happen. You can't make assumptions on how people will use this, and likewise, you can't force people not to use the Classic Block if they want to.

That said, media in the classic block is very different than media in Gutenberg.

How is the media different? When you insert an Inline Image Block, all it does is that it allows you to select an image and then it inserts the appropriate <img> tag into the Paragraph Block. How is this different to what you're going to be doing for the Classic Block? You're simply going to insert an image in exactly the same way. They're not different. It's exactly the same thing. The only difference is how you go about inserting the image. The end results though, is identical. This is what a classic block looks like after inserting and Inline Image block...

screenshot_872

It's not possible to add any media (image, gallery, audio...) Gutenberg blocks inside the Classic block, so it makes more sense for the button to be more prominent.

Nowhere have I suggested that it should be possible to insert Gutenberg Blocks inside a classic block. All I've suggested is that you add a media button to the Paragraph block so that people can insert inline images in exactly the same way as they'll be inserting them into the Classic Block, rather than making them insert an Inline Image Block. Having a media button on the toolbar is going to be a significantly easier experience for the end user, than having to Insert and Inline Image block, and it keeps the ui more consistent.

Adding inline images inside the normal Gutenberg blocks should be much rarer while we have dedicated blocks that you can insert at the block level.

This is completely inaccurate. Anyone who writes documentation will use inline images. If you're describing the use of an icon, for example, it helps to have an small image of the icon inline so your readers know exactly which icon you're referring to.

Inline images are really just inserted in the line of text and meant to be small.

I fully realise that but that really doesn't have anything to do with anything that we've been discussing.

Based on your comments above, it's clear that you have no intention of changing it at this point, but personally, I think that's a mistake and it's only making the ui harder and more complicated for the end user.

@tofumatt

This comment has been minimized.

Show comment
Hide comment
@tofumatt

tofumatt Oct 5, 2018

Member

We won't be adding a media inserter/inline media to the paragraph block. As @iseulde mentioned: it's expected that you would use a dedicated block for that; inline images are a bit of a special-case scenario. The paragraph block is for paragraph content, and the point of blocks in general is to be discrete units of content, not ones that are mixed. A paragraph block with inline media and images would be a Classic block 😄

I don't think further discussion on this issue will be fruitful, so I'm going to lock this issue for now.

Member

tofumatt commented Oct 5, 2018

We won't be adding a media inserter/inline media to the paragraph block. As @iseulde mentioned: it's expected that you would use a dedicated block for that; inline images are a bit of a special-case scenario. The paragraph block is for paragraph content, and the point of blocks in general is to be discrete units of content, not ones that are mixed. A paragraph block with inline media and images would be a Classic block 😄

I don't think further discussion on this issue will be fruitful, so I'm going to lock this issue for now.

@WordPress WordPress locked as resolved and limited conversation to collaborators Oct 5, 2018

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