Make WordPress Core

Opened 4 years ago

Closed 5 months ago

Last modified 5 months ago

#53681 closed defect (bug) (fixed)

Twenty Nineteen: reduce bottom margin for Audio block

Reported by: pgeorgiev's profile PGeorgiev Owned by: karmatosed's profile karmatosed
Milestone: 6.7 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: has-patch needs-refresh commit
Focuses: Cc:

Description

Audio in Twenty Nineteen are lacking a bottom margin

Posts that have audio: Cover block with background image is too close to the media.
Attached is a screenshot from Mozilla Firefox

Attachments (14)

twenty-nineteen-audio.png (570.5 KB) - added by PGeorgiev 4 years ago.
Twenty Nineteen audi file without margin
53681.diff (438 bytes) - added by PGeorgiev 4 years ago.
Screen Shot 2021-07-20 at 09.06.54.png (507.8 KB) - added by desrosj 4 years ago.
Screen Shot 2021-07-20 at 10.45.51.png (382.1 KB) - added by desrosj 4 years ago.
block-audio-patch.png (627.9 KB) - added by PGeorgiev 4 years ago.
figure.wp-block-audio
53681.2.diff (468 bytes) - added by PGeorgiev 4 years ago.
Added patch 53681.2
Screenshot at Jun 12 11-56-04 PM.png (931.3 KB) - added by hmbashar 8 months ago.
Without testing any patch, here is my default view
2019-audio-front-before.png (586.9 KB) - added by sabernhardt 6 months ago.
front end before patch
2019-audio-front-with-patch.png (584.4 KB) - added by sabernhardt 6 months ago.
front end with patch
2019-audio-iframe-before.png (591.9 KB) - added by sabernhardt 6 months ago.
iframe editor before patch
2019-audio-iframe-with-patch.png (590.9 KB) - added by sabernhardt 6 months ago.
iframe editor with patch
2019-audio-nonframe-before.png (597.7 KB) - added by sabernhardt 6 months ago.
non-framed editor before patch
2019-audio-nonframe-with-patch.png (597.9 KB) - added by sabernhardt 6 months ago.
non-framed editor with patch
2019-audio-blocks-comparison.png (60.0 KB) - added by sabernhardt 6 months ago.
side-by-side comparison before and after applying patch

Change History (33)

@PGeorgiev
4 years ago

Twenty Nineteen audi file without margin

@PGeorgiev
4 years ago

#1 @SergeyBiryukov
4 years ago

  • Summary changed from Audio in Twenty Nineteen are lacking bottom margin to Twenty Nineteen: Audio are lacking bottom margin

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


4 years ago

#3 follow-up: @desrosj
4 years ago

  • Keywords reporter-feedback added
  • Version trunk deleted

Hi @PGeorgiev,

Thanks for this ticket! I'm having a little bit of trouble understanding the issue as reported.

I've attached what I am seeing when using Twenty Nineteen and inserting an audio block followed by a cover block. There appears to be a proper margin between the two inherited from the .entry .entry-content > * CSS rule.

Is the suggestion that there should be less of a margin after an audio block? Based on the patch applying a -24px margin, I think that's the case. But I'm not sure that's right because every block has a margin-bottom of 32px.

#4 in reply to: ↑ 3 @PGeorgiev
4 years ago

Replying to desrosj:

Hi @PGeorgiev,

Thanks for this ticket! I'm having a little bit of trouble understanding the issue as reported.

I've attached what I am seeing when using Twenty Nineteen and inserting an audio block followed by a cover block. There appears to be a proper margin between the two inherited from the .entry .entry-content > * CSS rule.

Is the suggestion that there should be less of a margin after an audio block? Based on the patch applying a -24px margin, I think that's the case. But I'm not sure that's right because every block has a margin-bottom of 32px.

Thanks for your reply,

It's just that the space between the two blocks seemed really big to me and that's why I'm proposing to reduce it.

Sorry, this is my first ticket (maybe a second) and if you don't think it makes sense, you can close it of course.

Version 0, edited 4 years ago by PGeorgiev (next)

#5 @desrosj
4 years ago

  • Focuses ui removed
  • Keywords needs-testing needs-patch added; has-patch reporter-feedback removed

Thanks for clarifying! I understand now.

I think that the margin is correct. I took a second look, and the space is definitely larger than between other blocks. It seems there is a height or line-height somewhere for the <figure> or <audio> tags resulting in white space.

I think the right way forward is to figure that out rather than adjusting the margin.

@PGeorgiev
4 years ago

figure.wp-block-audio

@PGeorgiev
4 years ago

Added patch 53681.2

#6 @PGeorgiev
4 years ago

Hey @desrosj,
I apologize a lot for the delay here. I added a style that only applies to figure.wp-block-audio in this case will only apply to this block and we will not affect the styles of the others

I would be happy to share your feedback as this is my first ticket

#7 @karmatosed
10 months ago

  • Keywords has-patch added; needs-testing needs-patch removed

#8 @karmatosed
10 months ago

  • Milestone changed from Awaiting Review to Future Release

This ticket was mentioned in PR #6575 on WordPress/wordpress-develop by @sabernhardt.


9 months ago
#9

Sets audio element to display: block in the Audio block, for both the front end and the editor.

Trac 53681

#10 @sabernhardt
9 months ago

  • Keywords needs-testing added
  • Summary changed from Twenty Nineteen: Audio are lacking bottom margin to Twenty Nineteen: reduce bottom margin for Audio block

#11 @karmatosed
8 months ago

  • Milestone changed from Future Release to 6.6

Moving to this release for consideration and testing.

@hmbashar
8 months ago

Without testing any patch, here is my default view

#12 @karmatosed
8 months ago

  • Milestone changed from 6.6 to Future Release

Putting back into future release as seems to need testing to see if still an issue or not.

#13 @karmatosed
6 months ago

  • Owner set to karmatosed
  • Status changed from new to assigned

Assigning to myself for testing.

#14 @karmatosed
6 months ago

@sabernhardt unless I am mistaken the patch on this doesn't have any changes to the display without patch. Are you able to have a little check to see?

#15 @karmatosed
6 months ago

  • Keywords needs-refresh added; needs-testing removed

@sabernhardt
6 months ago

front end before patch

@sabernhardt
6 months ago

front end with patch

@sabernhardt
6 months ago

iframe editor before patch

@sabernhardt
6 months ago

iframe editor with patch

@sabernhardt
6 months ago

non-framed editor before patch

@sabernhardt
6 months ago

non-framed editor with patch

#16 @sabernhardt
6 months ago

The screenshots are taken with Firefox/Windows, and I also saw a difference with the patch in Chrome/Windows. Maybe another browser/OS sets the audio element to block by default.

Side note: the caption styling is different in the iframe because the figcaption ruleset in style-editor does not have the .editor-styles-wrapper class, and now .wp-block-audio :where(figcaption) in block-library/theme.css has a higher specificity. On the front, the font size comes from style.css and the color from block-library/theme.css.

Last edited 6 months ago by sabernhardt (previous) (diff)

@sabernhardt
6 months ago

side-by-side comparison before and after applying patch

#17 @karmatosed
5 months ago

  • Keywords commit added
  • Milestone changed from Future Release to 6.7

Based on the above, I am going to move this to commit as have also found similar in testing after the steps were shown.

#18 @karmatosed
5 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 58959:

Twenty Nineteen: Reduce bottom margin for Audio block.

The audio block was lacking a bottom margin in some editors. This resolves both non-framed and framed changes.

Props PGeorgiev, desrosj, sabernhardt, hmbashar.
Fixes #53681.

@sabernhardt commented on PR #6575:


5 months ago
#19

Committed in r58959

Note: See TracTickets for help on using tickets.