WordPress.org

Make WordPress Core

Opened 8 months ago

Closed 3 months ago

#49231 closed defect (bug) (fixed)

Plugins admin screens: form fields issue on small screens

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

Description

Plugins admin screens: form fields issue on small screens

Attachments (22)

addplugin.png (19.0 KB) - added by passoniate 8 months ago.
alignmnet.png (25.6 KB) - added by passoniate 8 months ago.
common.css.patch (425 bytes) - added by garethgillman 8 months ago.
Adds margin to the select box when under 600px width
common.min.css.patch (34.9 KB) - added by passoniate 7 months ago.
Patch for RTL and LTR
list-tables.css.patch (129.0 KB) - added by passoniate 7 months ago.
Patch for RTL and LTR
common.css.2.patch (433 bytes) - added by passoniate 7 months ago.
common.css Patch Update
list-tables.css.2.patch (547 bytes) - added by passoniate 7 months ago.
list-tables.css Patch Update
49231.600.patch (881 bytes) - added by sabernhardt 7 months ago.
Sets search box top margin at 600px breakpoint and edits padding for select all checkbox
49231.782.patch (885 bytes) - added by sabernhardt 7 months ago.
Sets search box top margin at 782px breakpoint and edits padding for select all checkbox
49231.1.diff (1.0 KB) - added by maxpertici 7 months ago.
CSS Fix for plugins search form and table list
plugins-search-form-align.png (35.8 KB) - added by maxpertici 7 months ago.
Use inline-block for better alignment
new-margin.png (31.2 KB) - added by maxpertici 7 months ago.
Change margin logic. Use margin on form elements instead for wrapper
49231 on WordPress 5.1.png (156.5 KB) - added by afercia 7 months ago.
Search plugins input on WordPress 5.1 in the responsive view
49231.2.diff (1.5 KB) - added by maxpertici 6 months ago.
CSS Fix for plugins search form and table list with media queries
unaligned form.png (32.3 KB) - added by maxpertici 6 months ago.
unaligned form
49231.2.2.diff (1.5 KB) - added by audrasjb 3 months ago.
Patch refresh to address WPCS issues
49231.2.3.diff (1.5 KB) - added by sabernhardt 3 months ago.
refresh for WPCS
49231 strings length.png (279.8 KB) - added by afercia 3 months ago.
Patch applied: English vs. French
49231.diff (2.6 KB) - added by afercia 3 months ago.
49231.remove-br.diff (3.0 KB) - added by sabernhardt 3 months ago.
cancels float on search-plugins form element and removes <br class="clear">
49231.3.diff (3.5 KB) - added by afercia 3 months ago.
49231 various viewport widths.jpg (279.8 KB) - added by afercia 3 months ago.
Screenshots with admin language set to French (longer strings), at various viewport widths

Download all attachments as: .zip

Change History (58)

@passoniate
8 months ago

@passoniate
8 months ago

#1 @audrasjb
8 months ago

  • Focuses css added
  • Milestone changed from Awaiting Review to 5.3.3
  • Status changed from assigned to accepted

#2 @audrasjb
8 months ago

  • Keywords needs-patch added

Good catch. For the second issue, we should probably use:

@media screen and (max-width: 782px) {
    #wpbody-content .wp-list-table.plugins td
        padding: 10px 6px;
    }
}

Instead of

@media screen and (max-width: 782px) {
    #wpbody-content .wp-list-table.plugins td
        padding: 10px 9px;
    }
}

#3 @audrasjb
8 months ago

The first issue doesn't looks like a blocker to me. We may want to improve it if possible though.

#4 @audrasjb
8 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.

@garethgillman
8 months ago

Adds margin to the select box when under 600px width

#5 @audrasjb
7 months ago

Would be great to get a patch for both issues.

@passoniate
7 months ago

Patch for RTL and LTR

@passoniate
7 months ago

Patch for RTL and LTR

#6 @passoniate
7 months ago

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

#7 @audrasjb
7 months ago

  • Keywords needs-refresh added

Hi! Thank you for the patch!
RTL and minified versions of the file are generated automatically and don't need to be patched.

@passoniate
7 months ago

common.css Patch Update

@passoniate
7 months ago

list-tables.css Patch Update

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


7 months ago

#9 @afercia
7 months ago

This ticket was discussed during today's accessibility bug-scrub: it would be great to unify the various patches in just one for easier review and commit :)

#10 @audrasjb
7 months ago

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

Works fine on my side.
Marking this for commit.
Please note it should be committed as soon as possible or punted to 5.5 if we are not able to commit very soon :-)

#11 @sabernhardt
7 months ago

  • Keywords commit removed

@passoniate Thanks! The latest change to list-table.css looks good, but the search box's top margin is set twice in the common.css.2.patch.

Which top margin should we keep? I do not see a need for it at the 782px breakpoint. However, if I'm missing something and that should be at 782, then it's redundant to add the margin again at the 600px breakpoint.

I'll upload combined patches to review both options.

@sabernhardt
7 months ago

Sets search box top margin at 600px breakpoint and edits padding for select all checkbox

@sabernhardt
7 months ago

Sets search box top margin at 782px breakpoint and edits padding for select all checkbox

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


7 months ago

@maxpertici
7 months ago

CSS Fix for plugins search form and table list

#13 @maxpertici
7 months ago

Hi,
I propose a simpler fix.
What's it doing ?

1/ input and select are now aligned (inline-block on the parent label of the input)
2/ the margins (top) are directly applied to the elements to have a good space also in small screen (and the margin top of the wrapper is removed)

3/ slight correction of padding value on for table-list

See 49231.1.diff​.

@maxpertici
7 months ago

Use inline-block for better alignment

@maxpertici
7 months ago

Change margin logic. Use margin on form elements instead for wrapper

#14 @audrasjb
7 months ago

  • Keywords has-screenshots commit added

Patch looks good to me, thank you @maxpertici.

Ping @afercia : it's our last window for this bugfix to land in 5.4.
In my opinion, it looks pretty self contained and good to go before RC1. Thoughts?

Last edited 7 months ago by audrasjb (previous) (diff)

#15 @afercia
7 months ago

  • Keywords commit removed
  • Version 5.3.2 deleted

Looking at 49231.1.diff

  • setting display: inline-block on the label alters the width of the inner search input: on the small screens view, the search input is intentionally set to have a width of 100% but now it's not full-width any longer because its container is inline-block
  • the fix for the "Select all" checkbox table cell alters the td elements also on the desktop view
  • both fixes should be applied only for the responsive view at the appropriate breakpoint

49231.782.patch seems preferable to me because it changes the CSS only in the responsive view but:

  • .wp-filter .search-form input[type="search"] is used also in the "Add Themes" page where there's now an unnecessary top margin
  • it changes all the td elements in the plugins table including the ones with the plugin names thus reducing the horizontal spacing: it should target only the "Select all" table cell

Noting that seems to me both these visual glitches happen since a long time, see the attached screenshot taken on WP 5.1.

@afercia
7 months ago

Search plugins input on WordPress 5.1 in the responsive view

#16 @afercia
7 months ago

P.S. I feel physical pain looking at that wrapping label :) Maybe we should consider a larger improvement and use a non-wrapping label with associated to the search field with for / id attributes, as per the WordPress accessibility coding standards. At this point of the release, that should happen in the next development cycle though.

#17 @audrasjb
7 months ago

  • Keywords needs-refresh added
  • Milestone changed from 5.4 to 5.5

Thanks for the feedback @afercia

Moving this ticket to WP 5.5 for further investigation/testing.

@maxpertici
6 months ago

CSS Fix for plugins search form and table list with media queries

@maxpertici
6 months ago

unaligned form

#18 @maxpertici
6 months ago

I suggest a second path 49231.2.diff following comments from @afercia. I adjusted the behavior of the search field on responsive view and I also adjusted the behavior of the form (see screenshot).
I also modify my css for table list checkbox.

#19 @maxpertici
6 months ago

  • Keywords needs-refresh removed

#20 @audrasjb
5 months ago

  • Keywords early added

Patch looks good to me.
@afercia I think it would be nice to commit it early when you have a chance, so it would be easier to test it more closely.

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


5 months ago

#22 @afercia
5 months ago

  • Keywords needs-refresh added

Looking at 49231.2.diff it would need some coding standards. Things like:

  • indentation should use tab characters
  • space between CSS selector and opening curly brace

For a full reference, please see https://make.wordpress.org/core/handbook/best-practices/coding-standards/css/

Also, not sure about the approach because it's changing all the .check-column. I'd also take this opportunity to change the wrapping label <label><input /></label> to a non-wrapping label with a for attribute.

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


5 months ago

#24 @audrasjb
5 months ago

This ticket was discussed during today’s bug scrub. @maxpertici can you please try to refresh your patch according to @afercia’s comment. Thanks!

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


4 months ago

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


3 months ago

@audrasjb
3 months ago

Patch refresh to address WPCS issues

#27 @audrasjb
3 months ago

  • Keywords early needs-refresh removed

@afercia I refreshed the last patch to follow WP coding standards.
Not sure about touching the wrapped label. I think it should be another ticket (and I'd be happy to open it for 5.6 :D ).

Removing early keyword as it's not super legit.

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


3 months ago

#29 @audrasjb
3 months ago

  • Keywords commit added

Adding commit keyword as the previous patch looks good to go.

#30 @sabernhardt
3 months ago

49231.2.2.diff still has spaces instead of tabs and lacks a space before the opening curly brace for two items in common.css.

I'll look more in-depth and should have an update shortly.

#31 @audrasjb
3 months ago

I don't know what I've done with that patch. I remember myself using linting on that patch… :( Thanks for spotting this @sabernhardt

@sabernhardt
3 months ago

refresh for WPCS

#32 @afercia
3 months ago

  • Keywords commit removed

Looking at 49231.2.3.diff I'm not sure the breakpoint at max-width: 1050 would work well.

1050 pixels seems to me a "magic number" (i.e. a number that isn't based on a logic condition) that works only when the admin language is set to English.

When the admin language is other from English, the strings used in the user interface will have a different length. When longer, the layout change won't work well as it should happen a a larger breakpoint than 1050. For example, see the attached screenshot with a comparison between English and French.

Seems to me a good case where using CSS flexbox would be helpful.

@afercia
3 months ago

Patch applied: English vs. French

@afercia
3 months ago

#33 @afercia
3 months ago

49231.diff builds on the previous patches and does a few things:

  • removes the wrapping label in favor of a label associated with for/id attributes, which is a win for accessibility
  • uses CSS flexbox for the layout of the "filter links" and the search form
  • groups some new CSS rulesets within the existing 782px media query instead of adding new ones as it's always best to reuse existing media queries

I've tested it on macOS browsers and flexbox seems to work well. Some more testing would be nice :)

@sabernhardt
3 months ago

cancels float on search-plugins form element and removes <br class="clear">

#34 @sabernhardt
3 months ago

I checked Windows browsers (Chrome, Firefox, Edge, IE11), and in multiple languages this time (including Arabic, French and Hebrew).

Switching to flexbox works for me, though it adds extra space below the wp-filter bar.

If the float on .wp-filter .search-form.search-plugins is cancelled, the <br class="clear"> should be unnecessary.

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

@afercia
3 months ago

#35 @afercia
3 months ago

  • Keywords commit added

Thanks @sabernhardt good catch. It's a pleasure to finally have the opportunity to start removing those <br class="clear" /> :)

49231.3.diff

  • adds align-items: center; to improve vertical centering of the search form
  • float: none is not necessary as flexbox will ignore floats on flex items
  • removes the CSS class install-help from the paragraph "If you have marked plugins as favorites ..." to make the margin consistent and to remove italic type, see #47327

@afercia
3 months ago

Screenshots with admin language set to French (longer strings), at various viewport widths

#36 @afercia
3 months ago

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

In 48281:

Accessibility: Plugins: Accessibility and CSS improvements for the Plugins pages.

  • improves checkboxes alignment on the "Plugins" page table in the responsive view
  • improves spacing between form controls on the "Add Plugins" page in the responsive view
  • the layout of the "filter bar" on the "Add Plugins" page is now based on CSS Flexbox
  • removes italic type from a paragraph in the "Favorites" page

Props passoniate, garethgillman, maxpertici, audrasjb, sabernhardt, afercia.
See #47327.
Fixes #49231.

Note: See TracTickets for help on using tickets.