Make WordPress Core

Opened 7 years ago

Closed 5 years ago

Last modified 5 years ago

#42938 closed feature request (fixed)

Avatar Blank Space

Reported by: marius84's profile Marius84 Owned by: ianbelanger's profile ianbelanger
Milestone: 5.4 Priority: normal
Severity: normal Version: 4.9.1
Component: Administration Keywords: has-screenshots has-patch commit
Focuses: ui, administration Cc:

Description

Wordpress --> Settings --> Discussion --> Avatar Display --> Uncheck Show Avatar option.

If I can choose not to show the avatar the blank space shouldn’t be there.

screenshot 1: https://www.dropbox.com/s/3mj8fmsd3t9vycs/Screenshot%202017-12-19%2005.53.21.png?dl=0

screenshot 2: https://www.dropbox.com/s/1ip4fmozaub5nyw/Screenshot%202017-12-19%2013.49.19.png?dl=0

screenshot 3: https://www.dropbox.com/s/9013zsym8v8gltc/Screenshot%202017-12-19%2013.58.09.png?dl=0

Also all themes will show on live website this "Blank Space Location On comment Area" if i deactivated the Avatar Display. If I can choose not to show the avatar the blank space location shouldn’t be there.

https://wordpress.org/support/topic/wordpress-4-9-1-bug-2/

Attachments (16)

Screenshot 2017-12-19 05.53.21.png (60.9 KB) - added by Marius84 7 years ago.
Screenshot 2017-12-19 13.49.19.png (54.4 KB) - added by Marius84 7 years ago.
Screenshot 2017-12-19 13.58.09.png (46.1 KB) - added by Marius84 7 years ago.
42938.patch (995 bytes) - added by Shital Patel 7 years ago.
Created patch for Avatar in Recent Comments section on Dashboard
42938.1.patch (1.0 KB) - added by Shital Patel 7 years ago.
42938-before-patch.PNG (17.7 KB) - added by ianbelanger 6 years ago.
Before applying patch
42938-after-patch.PNG (18.3 KB) - added by ianbelanger 6 years ago.
After applying patch
42938-3.diff (1.5 KB) - added by sgastard 6 years ago.
42938.4.diff (3.1 KB) - added by ianbelanger 6 years ago.
42938.5.diff (1.7 KB) - added by garrett-eclipse 5 years ago.
Refresh
42938.6.diff (1.7 KB) - added by garrett-eclipse 5 years ago.
One tiny tweak, moved the space off the $show_avatars_class setup and just placed after <?php echo $show_avatars_class; ?> as it will always be needed
Capture d’écran 2020-01-17 à 23.26.07.png (67.1 KB) - added by audrasjb 5 years ago.
Testing last patch: works fine.
42938.7.diff (1.8 KB) - added by audrasjb 5 years ago.
Refreshed patch
Screen Shot 2020-01-17 at 3.49.30 PM.png (31.4 KB) - added by garrett-eclipse 5 years ago.
Before
Screen Shot 2020-01-17 at 3.48.10 PM.png (30.6 KB) - added by garrett-eclipse 5 years ago.
After
42938.8.diff (1.8 KB) - added by audrasjb 5 years ago.
New patch to address the LF/CRLF issue reported by Garrett

Download all attachments as: .zip

Change History (44)

@Shital Patel
7 years ago

Created patch for Avatar in Recent Comments section on Dashboard

#1 @williampatton
7 years ago

  • Type changed from defect (bug) to enhancement

#2 @Shital Patel
7 years ago

  • Focuses ui administration coding-standards added
  • Keywords has-patch added

#3 @GaryJ
7 years ago

  • Focuses coding-standards removed

This isn't related to adhering to coding standards, so removing the focus.

Identification of the bug looks good, assuming that the intent of the Show Avatar was meant for the back end, as well as the front end.

#4 @SergeyBiryukov
7 years ago

  • Milestone changed from Awaiting Review to 5.0

#5 @johnbillion
6 years ago

  • Milestone changed from 5.0 to 5.1

#6 follow-up: @Marius84
6 years ago

Hello, WP 5.0 installed problem persist, any update on this? Thank you all.

#7 in reply to: ↑ 6 @knutsp
6 years ago

Replying to Marius84:

Hello, WP 5.0 installed problem persist, any update on this? Thank you all.

As you can see by the milestone field in the ticket header, and also previous comment, this is intended to be fixed in WordPress 5.1.

#8 @pento
6 years ago

  • Keywords needs-testing added
  • Milestone changed from 5.1 to 5.2

42938.1.patch needs testing and a decision.

#9 @Marius84
6 years ago

My wordpress 5.0.3 admin dashboard look horrible just because I don't use Avatar. Hope we can have some news about this subject in wordpress 5.2
Thanks guys! I tested the patch and work like a charm!

Last edited 6 years ago by Marius84 (previous) (diff)

#10 @Shital Patel
6 years ago

  • Type changed from enhancement to feature request

@ianbelanger
6 years ago

Before applying patch

@ianbelanger
6 years ago

After applying patch

#11 follow-up: @ianbelanger
6 years ago

  • Keywords needs-patch has-screenshots added; has-patch removed
  • Milestone changed from 5.2 to 5.3

Just tested the patch and it does make the requested changes, however there is an unintended consequence when your comment contains a long non-breaking word/string. See screenshots above.

Since this is not ready for 5.2 Beta 1, which is tomorrow and is a Feature Request. I am punting to 5.3

@sgastard
6 years ago

#12 in reply to: ↑ 11 @sgastard
6 years ago

  • Keywords has-patch added; needs-patch removed

Replying to ianbelanger:

Just tested the patch and it does make the requested changes, however there is an unintended consequence when your comment contains a long non-breaking word/string. See screenshots above.

Since this is not ready for 5.2 Beta 1, which is tomorrow and is a Feature Request. I am punting to 5.3

Add new class dashboard-comment-wrap-no-avatar to keep the word-wrap property

#13 @lgrev01
6 years ago

Just tested the patch and it does make de requested changes, but when I go back and want to show the avatars again. It doesn't show the avatar anymore.

#14 @lgrev01
6 years ago

  • Keywords needs-testing removed

@ianbelanger
6 years ago

#15 @ianbelanger
6 years ago

Just tested the patch and it does make de requested changes, but when I go back and want to show the avatars again. It doesn't show the avatar anymore.

42938.4.diff should fix the issue and also includes some coding standards updates.

#16 @donmhico
5 years ago

I can confirm that 42938.4.diff works. But I think we should also account the "No avatar" option on bundled themes for the front end as well. But that can be as another ticket.

#17 @desrosj
5 years ago

  • Keywords needs-refresh added

This one is not applying cleanly for me. Can someone please give it a refresh?

@garrett-eclipse
5 years ago

Refresh

#18 @garrett-eclipse
5 years ago

  • Keywords needs-refresh removed

@desrosj can you give 42938.5.diff a shot.

It's a refresh and addresses minor CS issues. One lucky happenstance was the CS fix on the main PHP block that appeared changed in 42938.4.diff was it's indention which is why it's no longer appearing in the latest diff as that doesn't change from what's in trunk.

@ianbelanger or @donmhico can you confirm as you have more background on this ticket.

As well @donmhico did you create another ticket for this thought?
"But I think we should also account the "No avatar" option on bundled themes for the front end as well. But that can be as another ticket."
*If so please post the reference for relation.

Thanks

@garrett-eclipse
5 years ago

One tiny tweak, moved the space off the $show_avatars_class setup and just placed after <?php echo $show_avatars_class; ?> as it will always be needed

#19 @desrosj
5 years ago

  • Milestone changed from 5.3 to 5.4

With 5.3 beta 1 in a few hours, this unfortunately needs to be punted.

#20 @Marius84
5 years ago

I hoped in vain that it would be solved in WordPress 5.3 and now the milestone has been moved to 5.4 a real disappointment

#21 @audrasjb
5 years ago

  • Keywords needs-refresh added

The patch doesn't applies cleanly anymore on my side. Going to refresh it.

@audrasjb
5 years ago

Testing last patch: works fine.

@audrasjb
5 years ago

Refreshed patch

#22 @audrasjb
5 years ago

  • Keywords needs-refresh removed
  • Owner set to audrasjb
  • Status changed from new to accepted

Patch refreshed. Works fine on my side, I think it's ready to go.

#23 @garrett-eclipse
5 years ago

  • Keywords commit added

Thanks for the refresh @audrasjb testing this is working nicely. I think we're safe marking this for commit.

One note about your latest patch, I'm not sure why it does this really but am getting;

patch unexpectedly ends in middle of line
Hunk #2 succeeded at 808 with fuzz 1.

Seems this is just a CRLF windows line ending from a git diff that it's complaining about but the patch applies cleanly without issue so I don't see it as a concern, just flagging in case.

@audrasjb
5 years ago

New patch to address the LF/CRLF issue reported by Garrett

#24 @audrasjb
5 years ago

Hi there,

I verified the previous patch and it still applies cleanly.
It’s still on track for 5.4, we just need a commit action here.

#25 @ianbelanger
5 years ago

  • Owner changed from audrasjb to ianbelanger
  • Status changed from accepted to assigned

Patch looks good to me, reviewing for commit

#26 @ianbelanger
5 years ago

Since this is not a theme ticket, I cannot commit it, but the patch looks good. @desrosj can you get this in?

#27 @SergeyBiryukov
5 years ago

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

In 47255:

Administration: Remove a blank space in the Recent Comments dashboard widget if avatars are disabled on Discussion Settings screen.

Props Marius84, shital-patel, GaryJ, ianbelanger, sgastard, lgrev01, donmhico, garrett-eclipse, audrasjb, SergeyBiryukov.
Fixes #42938.

#28 @SergeyBiryukov
5 years ago

In 47256:

Administration: Move .has-row-actions class in Recent Comments dashboard widget next to .dashboard-comment-wrap, for consistency.

Follow-up to [47255].

See #42938.

Note: See TracTickets for help on using tickets.