WordPress.org

Make WordPress Core

Opened 4 months ago

Last modified 10 days ago

#49228 reopened defect (bug)

Widgets: Checkbox overlap on small screens

Reported by: passoniate Owned by: audrasjb
Milestone: 5.5 Priority: normal
Severity: normal Version: 5.3.2
Component: Widgets Keywords: has-screenshots good-first-bug has-patch commit
Focuses: ui, accessibility, css Cc:

Description

Checkbox overlap need to margin-top

Attachments (9)

checkboxoverlap.png (22.5 KB) - added by passoniate 4 months ago.
On mobile device
Capture d’écran 2020-01-19 à 14.45.58.png (23.7 KB) - added by audrasjb 4 months ago.
49228.diff (391 bytes) - added by milindmore22 4 months ago.
Fixes overlapping checkboxes and radio buttons on Chrome Safari and Firefox Browsers on Responsive Screen
wp 5.1.png (84.6 KB) - added by afercia 4 months ago.
49228.1.diff (4.3 KB) - added by jaz_on 6 weeks ago.
New patch.
49228.2.diff (433 bytes) - added by SergeyBiryukov 6 weeks ago.
49228.2.png (11.6 KB) - added by SergeyBiryukov 6 weeks ago.
Befoew patch.png (16.4 KB) - added by mukesh27 5 weeks ago.
After patch.png (17.9 KB) - added by mukesh27 5 weeks ago.

Download all attachments as: .zip

Change History (33)

@passoniate
4 months ago

On mobile device

#1 @audrasjb
4 months ago

  • Keywords reporter-feedback added

Hi, thanks for opening this ticket,

Could you please give some context for this issue?
Would be nice to describe the issue and where it happens (which admin screen, etc.) and on which browser.
It would also be very helpful to provide screenshots for the issue :-)

Cheers,
Jb

#2 @audrasjb
4 months ago

  • Keywords needs-patch added; reporter-feedback removed
  • Milestone changed from Awaiting Review to 5.3.3
  • Status changed from assigned to accepted

Thanks @passoniate I can reproduce the issue on my side.
Let's handle that in the next minor as it's a regression from 5.3.

#3 @audrasjb
4 months ago

  • Summary changed from Checkbox overlap to Widgets: Checkbox overlap on small screens

#4 @audrasjb
4 months ago

This issue occurs only on Categories and Archives widgets.
I think the easier solution is to use proper paragraph wrapper for each input line.

#5 @audrasjb
4 months ago

  • Keywords good-first-bug added

#6 @audrasjb
4 months ago

Adding good-first-bug keyword, which means that experimented contributors (= people that already contributed to a core patch) should ideally leave few days for first timers to give it a try before adding a patch.

@milindmore22
4 months ago

Fixes overlapping checkboxes and radio buttons on Chrome Safari and Firefox Browsers on Responsive Screen

#7 @milindmore22
4 months ago

  • Keywords has-patch added; needs-patch removed

#8 @audrasjb
4 months ago

  • Milestone changed from 5.3.3 to 5.4

Moving all unfixed tickets from 5.3.3 to milestone 5.4, as there is no plan for a 5.3.3 maintenance release for now.

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


4 months ago

#10 @afercia
4 months ago

This ticket was discussed during today's accessibility bug-scrub. Noticed the checkboxes slightly overlap now on 5.3. On all previous versions a vertical spacing was missing as well, even if there was no overlap. See attached screenshot from WP 5.1.

@afercia
4 months ago

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


3 months ago

#12 @afercia
3 months ago

  • Keywords needs-testing added

This ticket was discussed during today's accessibility bug-scrub: the proposed patch appears to target all checkboxes and radio buttons on small screens. Needs to be reviewed carefully as this will change the margins for all these elements within paragraphs.

#13 @audrasjb
3 months ago

  • Milestone changed from 5.4 to 5.5

Hi,

Given the last comment and with 5.4 Beta 3 approaching and the Beta period reserved for bugs introduced during the cycle, this is being moved to WP 5.5.

#14 @carriganvb
3 months ago

@audrasjb I've been looking at this. It seems to me that there are two approaches on the table to solving this. @milindmore22's approach was to change the style causing the squishing of the checkboxes directly. Your suggested approach (if I'm interpreting it correctly) is to surround each row in its own <p> tag, like this:

<p>
  <input1>
  <label1>
</p>
<p>
  <input2>
  <label2>
</p>
<p>
  <input3>
  <label3>
</p>

As opposed the current layout:

<p>
  <input1>
  <label1>
  <br1>
  <input2>
  <label2>
  <br2>
</p>

Am I interpreting your suggested approach correctly? Is your approach still how you'd like to proceed, as opposed to a direct style change?

Thanks,
-Carrigan

#15 @audrasjb
6 weeks ago

  • Keywords needs-refresh added

@carriganvb yes, so this way, we don't have to worry about side effects on all the admin styles.

@jaz_on
6 weeks ago

New patch.

#16 @jaz_on
6 weeks ago

  • Resolution set to invalid
  • Status changed from accepted to closed

Hi, this is my first contribution : new patch proposal thanks to @audrasjb help.
Mainly adding <p> and deleting <br />.
Is there anyone who can check and give their opinion on this?
Thanks

Edit : not sure why "resolution" and "status" were set to "invalid" and "accepted to closed". I maybe made a mistake when sending my message... sorry.

Last edited 6 weeks ago by jaz_on (previous) (diff)

#17 @afercia
6 weeks ago

  • Keywords needs-refresh removed
  • Resolution invalid deleted
  • Status changed from closed to reopened

Restoring resolution and status.

#18 @SergeyBiryukov
6 weeks ago

Thanks for the patches!

Adding paragraphs is a good workaround, but that wouldn't fix the appearance for any custom widgets that might have the same issue.

Given that this issue doesn't happen on the Discussion Settings screen, which uses the same layout as the current Widgets screen (checkboxes and labels not wrapped in paragraphs, but with some extra margin), I think a similar approach could be used here to achieve the same effect.

See 49228.2.diff and 49228.2.png.

#19 @audrasjb
5 weeks ago

  • Keywords commit added; needs-testing removed

The last approach makes sense to me @SergeyBiryukov.
It works fine and it won't apply to other admin pages. Thanks.

#20 @SergeyBiryukov
5 weeks ago

  • Focuses css added

#21 @SergeyBiryukov
5 weeks ago

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

In 47598:

Accessibility: Widgets: Prevent checkboxes in widget control forms from overlapping on smaller screens.

Props jaz_on, milindmore22, passoniate, audrasjb, carriganvb, afercia, SergeyBiryukov.
Fixes #49228.

#22 follow-up: @mukesh27
5 weeks ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Hi @SergeyBiryukov please check the attached screenshot for the issue.

The search widget got an issue after the changes in desktop and mobile devices.

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

@mukesh27
5 weeks ago

@mukesh27
5 weeks ago

#23 in reply to: ↑ 22 @SergeyBiryukov
5 weeks ago

Replying to mukesh27:

The search widget got an issue after the changes in desktop and mobile devices.

Thanks for catching that! I cannot reproduce the issue in current trunk though, I think it was fixed in [47593], which moved the input out of the <label> tag in that widget for consistency with others.

That said, let's see if we can also make it work with the previous markup (with the input inside the <label> tag) for any custom widgets out there.

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


10 days ago

Note: See TracTickets for help on using tickets.