WordPress.org

Make WordPress Core

Opened 8 weeks ago

Closed 3 weeks ago

Last modified 13 days ago

#52287 closed defect (bug) (fixed)

Twenty Twenty One: Inline Images displaying on new lines (as display: block)

Reported by: talldanwp Owned by: peterwilsoncc
Milestone: 5.7 Priority: normal
Severity: normal Version: 5.6
Component: Bundled Theme Keywords: good-first-bug has-patch commit
Focuses: css Cc:

Description (last modified by SergeyBiryukov)

In Twenty Twenty One, inline images are display: block instead of display: inline-block.

This is the case both in the editor and in the front-end.

There's a similar ticket here for Twenty Twenty that could be worth fixing at the same time: #50418

Steps to reproduce:

  1. Add a paragraph block
  2. Type some text
  3. Add an inline image

Expected behavior:
The image displays on the same row as the text.

Actual behavior:
The image displays on a new line.

Attachments (10)

Inline image test.png (76.2 KB) - added by mukesh27 8 weeks ago.
Screenshot 2021-01-13 at 4.34.11 pm.png (24.2 KB) - added by talldanwp 8 weeks ago.
Reporter screenshot
with-pr-874.png (115.1 KB) - added by poena 7 weeks ago.
With the current PR applied:
Screen Shot 2021-02-03 at 12.18.57.png (56.9 KB) - added by paaljoachim 5 weeks ago.
Applied the patch. Inline image is still on a separate line.
Inline-image-2021.gif (336.9 KB) - added by paaljoachim 5 weeks ago.
Working patch for adding an inline image.
52287-vertical-align-middle.patch (27.5 KB) - added by poena 4 weeks ago.
Display: block is removed, but the alignment is kept
inline-image-vertical-align-middle.png (253.8 KB) - added by poena 4 weeks ago.
Inline images with vertical-align middle
52287-no-vertical-align.patch (27.2 KB) - added by poena 4 weeks ago.
Display: block and the alignment is removed
inline-image-no-vertical-align.png (248.9 KB) - added by poena 4 weeks ago.
Inline images without vertical-align middle
52287.patch (1.0 KB) - added by poena 3 weeks ago.
Removes display:block and duplicate CSS.

Download all attachments as: .zip

Change History (32)

#1 @mukesh27
8 weeks ago

In my test on admin inline image working fine.

#2 @talldanwp
8 weeks ago

@mukesh27 Is that Twenty Twenty One? It doesn't look like the same theme, but granted you might have changed some options.

I'll upload an example of what I see.

@talldanwp
8 weeks ago

Reporter screenshot

This ticket was mentioned in PR #874 on WordPress/wordpress-develop by gonzalezlopezjm.


7 weeks ago

  • Keywords has-patch added; needs-patch removed

Prevents that an inline image inserted into editor goes into new line in the TwentyTwentyOne Theme

Trac ticket: https://core.trac.wordpress.org/ticket/52287

#4 @poena
7 weeks ago

Hi!
I have tested the pull request. I did not see any side effects except the following:

There is a style in the image block file that affects all images, not only the block: sass/05-blocks/image/_style.scss#L41

This is duplicating the height and the max width that is already added in the sass/04-elements/media.scss file,
and it is adding a vertical-align:middle to the images on the front.
Because of the vertical alignment, the inline images do not look the same in the editor and front when the PR is applied.

I do not know the reason why the vertical alignment was included, further testing is needed to see if removing it has any side effects.

The left image is in the editor, right is on the front.

Last edited 7 weeks ago by poena (previous) (diff)

@poena
7 weeks ago

With the current PR applied:

#5 @SergeyBiryukov
7 weeks ago

  • Description modified (diff)

#6 @mukesh27
6 weeks ago

  • Keywords needs-testing added

Agree with @poena either we have to remove that vertical alignment from the front CSS or we have to include vertical alignment in editor CSS to make the same style on both front and editor.

needs-testing keyword added for more testing.

#7 follow-up: @paaljoachim
5 weeks ago

Tested a local site with Twenty Twenty theme.
Added some text to a paragraph block.
Added an inline image.
Noticed it showed up on a separate line.

Testing the patch.

Command:

npm run grunt patch:https://github.com/WordPress/wordpress-develop/pull/874

I noticed no difference from before and after applying the above.

I also added

npm run build

To see if that would help. No difference.
I assume I am doing something wrong.

#8 in reply to: ↑ 7 @poena
5 weeks ago

Replying to paaljoachim:

Tested a local site with Twenty Twenty theme.
Added some text to a paragraph block.
Added an inline image.
Noticed it showed up on a separate line.

Testing the patch.

Command:

npm run grunt patch:https://github.com/WordPress/wordpress-develop/pull/874

I noticed no difference from before and after applying the above.

I also added

npm run build

To see if that would help. No difference.
I assume I am doing something wrong.

This ticket is for Twenty Twenty-One, not Twenty Twenty, but maybe the above was a typo?

#9 @paaljoachim
5 weeks ago

Hi @poena

That is correct. It was a typo. I am testing with Twenty Twenty One. I am retesting now and applied the patch and even rebuilt the local environment.

I write some text and in between the text add an inline image.

This is what I see.

Last edited 5 weeks ago by paaljoachim (previous) (diff)

@paaljoachim
5 weeks ago

Applied the patch. Inline image is still on a separate line.

#10 @poena
5 weeks ago

Did you run npm build inside the twentytwentyone folder?

If you select the image in the editor and look at the source (in the browser dev tools),
does it say

.editor-styles-wrapper img {
    display: inline-block;
    height: auto;
    max-width: 100%;
}
Last edited 5 weeks ago by poena (previous) (diff)

#11 @paaljoachim
5 weeks ago

Went into:

cd wordpress-develop

Added:

git reset --hard

Added:

npm run grunt patch:https://github.com/WordPress/wordpress-develop/pull/874

Added:

npm run dev

Checked http://localhost:8889/
Noticed that the image even a very small one was added to a seperate line.

Checking the Chrome console.
It did not say inline-block. Which means I have not been able to test the patch yet.

CSS seen:

.editor-styles-wrapper img {
    display: block;
    height: auto;
    max-width: 100%;
}

---

Logged out of the localhost dev environment site.
Next back in terminal. I added:

npm run build:dev

Hmm I am still not able to successfully test the patch.
@hellofromtonya
Any thoughts?

This ticket was mentioned in Slack in #core by paaljoachim. View the logs.


5 weeks ago

#13 @paaljoachim
5 weeks ago

With help from @desrosj and @davidb in the core slack channel I got the patch working.
I added the instructions for testing 2019, 2020 and 2021 here:
https://meta.trac.wordpress.org/ticket/5581#comment:3

@paaljoachim
5 weeks ago

Working patch for adding an inline image.

#14 @jeroenrotty
4 weeks ago

I've tested the patch for Twenty Twenty One and can confirm together with @rolfsiebers that the inline-block patch does work as intended. Had some trouble finding the correct steps to build the theme as well, but found the correct steps thanks to @paaljoachim.

Last edited 4 weeks ago by jeroenrotty (previous) (diff)

This ticket was mentioned in Slack in #core by francina. View the logs.


4 weeks ago

#16 @poena
4 weeks ago

  • Keywords needs-refresh added

If I understand correctly the current patch has been tested, but not whether the
vertical-align:middle in comment #4 can be safely removed.

I will mark this as needs refresh.

@poena
4 weeks ago

Display: block is removed, but the alignment is kept

@poena
4 weeks ago

Inline images with vertical-align middle

@poena
4 weeks ago

Display: block and the alignment is removed

@poena
4 weeks ago

Inline images without vertical-align middle

#17 @poena
4 weeks ago

So, decisions, which one do we prefer. @melchoyce, any thoughts?

#18 @melchoyce
4 weeks ago

I personally think centered is a bit less awkward looking.

@poena
3 weeks ago

Removes display:block and duplicate CSS.

#19 @poena
3 weeks ago

  • Keywords commit added; needs-testing needs-refresh removed

Patch 52287.patch is a cleaner patch for inline images with vertical-align: middle, and only includes the changes to the scss files, not the built files.

#20 @peterwilsoncc
3 weeks ago

  • Owner set to peterwilsoncc
  • Resolution set to fixed
  • Status changed from new to closed

In 50351:

Twenty Twenty-One: Display inline-images inline.

Remove display: block property from inline images and display them center aligned vertically.

Props jeroenrotty, melchoyce, mukesh27, paaljoachim, poena, talldanwp.
Fixes #52287.

This ticket was mentioned in Slack in #core by monikarao. View the logs.


2 weeks ago

Note: See TracTickets for help on using tickets.