WordPress.org

Make WordPress Core

Opened 10 days ago

Last modified 14 hours ago

#48814 new defect (bug)

Comment marked as spam displays poorly in mobile view

Reported by: jeremyfelt Owned by:
Milestone: 5.4 Priority: normal
Severity: normal Version:
Component: Comments Keywords: good-first-bug has-patch needs-testing has-screenshots needs-design-feedback
Focuses: ui, css Cc:
PR Number:

Description

When a comment is marked as spam from wp-admin/edit-comments.php in a mobile view, the record of the comment with the option to undo is displayed in too small of an area rather than across the comment list table.

I believe this occurs because display of the .undo td is set to block in the list table CSS when it should be the default of table-cell.

Describing this is weird, so I'm attaching a GIF. :)

Attachments (8)

mobile-mark-as-spam.gif (71.7 KB) - added by jeremyfelt 10 days ago.
48814.diff (980 bytes) - added by lorenzof 9 days ago.
Hi, I created a patch for the ticket, it's my first patch so please check if everything is ok.
48814.1.diff (509 bytes) - added by lorenzof 9 days ago.
First patch, first mistake, I uploaded the wrong one, that's the right one
48814.2.diff (464 bytes) - added by mukesh27 8 days ago.
Patch.
48814.3.patch (459 bytes) - added by razamalik 7 days ago.
display:block change changed with display:table-cell
48814.3.2.patch (459 bytes) - added by razamalik 7 days ago.
display:block change changed with display:table-cell and i have tested in different browsers working fine! @lorenzof
48814-before.png (98.0 KB) - added by lorenzof 24 hours ago.
Comment display before
48814-after.png (109.1 KB) - added by lorenzof 24 hours ago.
Comment display after

Download all attachments as: .zip

Change History (18)

#1 @SergeyBiryukov
10 days ago

  • Keywords good-first-bug added
  • Milestone changed from Awaiting Review to Future Release

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


10 days ago

@lorenzof
9 days ago

Hi, I created a patch for the ticket, it's my first patch so please check if everything is ok.

@lorenzof
9 days ago

First patch, first mistake, I uploaded the wrong one, that's the right one

#3 @lorenzof
9 days ago

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

#4 @mukesh27
8 days ago

@lorenzof @jeremyfelt If we remove the display property from CSS .wp-list-table tr:not(.inline-edit-row):not(.no-items) td:not(.check-column) then it will fix the issue.

@mukesh27
8 days ago

Patch.

#5 @lorenzof
8 days ago

Thanks for the update @mukesh27

You removed the display property so that it inherits the table-cell value from td, and in that case there's no need to repeat the display: table-cell as I did my patch, right?

Last edited 8 days ago by lorenzof (previous) (diff)

#6 @mukesh27
8 days ago

@lorenzof Yes, it will take display property from td

@razamalik
7 days ago

display:block change changed with display:table-cell

@razamalik
7 days ago

display:block change changed with display:table-cell and i have tested in different browsers working fine! @lorenzof

#7 @lorenzof
7 days ago

@razamalik The .2 version of the patch submitted by @mukesh27 was fine and it's the best one since it's redundant to only change the value, so why you added a new version creating a patch that's the same one I submitted as .1 version, reverting his changes? .2 is fine, that's the one that need testing

#8 follow-up: @jeremyfelt
25 hours ago

  • Keywords needs-screenshots added

Thanks for the patches @lorenzof, @mukesh27, and @razamalik. If I follow correctly, it sounds like 48814.2.diff is the proposed solution—remove the display property for mobile?

It would also be helpful to have screenshots showing the change.

#9 in reply to: ↑ 8 @lorenzof
24 hours ago

Replying to jeremyfelt:

Thanks for the patches @lorenzof, @mukesh27, and @razamalik. If I follow correctly, it sounds like 48814.2.diff is the proposed solution—remove the display property for mobile?

It would also be helpful to have screenshots showing the change.

Yes, that's the right one. I appreciate the help from @razamalik who probably hasn't seen my patch but the right one it's the patch from @mukesh27. Less code, same result considering that the display: table-cell is inherited. I'll attach two screenshots from latest trunk.

@lorenzof
24 hours ago

Comment display before

@lorenzof
24 hours ago

Comment display after

#10 @jeremyfelt
14 hours ago

  • Keywords has-screenshots needs-design-feedback added; needs-screenshots removed
  • Milestone changed from Future Release to 5.4

Excellent, thanks @lorenzof! I'm going to add a design feedback tag here, but I think things are looking good for 5.4.

Note: See TracTickets for help on using tickets.