Make WordPress Core

Opened 4 months ago

Closed 3 months ago

Last modified 2 months ago

#60488 closed defect (bug) (fixed)

Plugins dependencies notice alters visual and DOM order in the plugin cards

Reported by: afercia's profile afercia Owned by: costdev's profile costdev
Milestone: 6.5 Priority: normal
Severity: normal Version: 6.5
Component: Plugins Keywords: has-screenshots has-patch commit
Focuses: ui, accessibility Cc:

Description

Noticed while testing https://core.trac.wordpress.org/ticket/22316

For accessibility, visual order and DOM order must alwasy match when they affect 'meaning and functionality'.

Basically, altering the visual order via CSS properties like order and the various flexbox/grid reverse properties must be avoided. It is only allowed for purely decorative elements like, for example, the position of an icon within a button (there was such a case in the Gutenberg editor).

The CSS recently added for the plugin dependencies alters the reading order of elements within the plugin cards in the 'Add Plugin' admin pave. See attached screenshot where I added numbers to illustrate the DOM order doesn't match the visual order.

Attachments (2)

order.png (203.8 KB) - added by afercia 4 months ago.
Screenshot 2024-02-11 at 8.19.40 AM.png (213.7 KB) - added by huzaifaalmesbah 3 months ago.
I think the "plugin-dependencies" section should always set the "plugin-card-top" button area.

Download all attachments as: .zip

Change History (21)

@afercia
4 months ago

#1 @costdev
4 months ago

  • Component changed from Upgrade/Install to Plugins
  • Milestone changed from Awaiting Review to 6.5
  • Owner set to costdev
  • Status changed from new to assigned

Thanks for reporting this @afercia!

I'll get working on a patch.

Moving to 6.5 for visibility and the Plugins component as it's more about the style and output of a plugin's data than it upgrade/installation.

#2 @afragen
4 months ago

  • Summary changed from Plugins dependencies notice alters visual and DOM order in teh plugin cards to Plugins dependencies notice alters visual and DOM order in the plugin cards

#3 @afercia
4 months ago

Sure, thanks @costdev and @afragen for the very fast feedback.

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


4 months ago
#4

  • Keywords has-patch added

In 57545, the notice for listing dependencies in a plugin card were styled with the CSS order properties. This created a mismatch between the visual order and DOM order of elements in the plugin card.

For accessibility, visual order and DOM order must always match when they affect meaning and functionality.

This removes the CSS order properties and outputs the dependencies notice later, making the visual and DOM order match.

### Before
https://i0.wp.com/github.com/WordPress/wordpress-develop/assets/79332690/b9b77e06-48a2-47a6-ac5a-da50db8140c8

### After
https://i0.wp.com/github.com/WordPress/wordpress-develop/assets/79332690/f68d7ec0-8d16-4798-8810-cd249970e7ab

#5 @costdev
4 months ago

  • Keywords needs-testing added

@afercia I've submitted PR 6078 which removes the CSS order properties and corrects the DOM order.

Please test 🙂 and remember to build as this makes CSS changes.

When you mention "the various flexbox/grid reverse properties", can you confirm whether this was just informative, or did you see any use of reverse properties by Plugin Dependencies? I want to make sure I don't miss anything.

Thanks!

@afragen commented on PR #6078:


3 months ago
#6

Tested. Works as designed. ✅

#9 @huzaifaalmesbah
3 months ago

Test Report

Patch tested: https://github.com/WordPress/wordpress-develop/pull/6078

Screenshots

Before Apply Patch After Apply Patch ✅
https://i.ibb.co/sRMjQ4D/Screenshot-2024-02-11-at-7-51-54-AM.png https://i.ibb.co/GFDVxQC/Screenshot-2024-02-11-at-7-50-17-AM.png

@huzaifaalmesbah
3 months ago

I think the "plugin-dependencies" section should always set the "plugin-card-top" button area.

#10 @afercia
3 months ago

When you mention "the various flexbox/grid reverse properties", can you confirm whether this was just informative, or did you see any use of reverse properties by Plugin Dependencies? I want to make sure I don't miss anything.

@costdev thanks. It was informative.

#11 @costdev
3 months ago

  • Keywords reporter-feedback added; needs-testing removed

Thanks @afercia!

Several testers have confirmed the patch works.

@afercia As the reporter, do you agree that PR 6078 resolves the reported issue and is ready for me to commit?

@afercia commented on PR #6078:


3 months ago
#12

The order is fixed so from an a11y perspective LGTM to me.

However, I noticed an unrelated issue: when the plugin name and description are short and take little vertical space, the plugin logo and the dependencies notice may overlap. Screenshot:

https://i0.wp.com/github.com/WordPress/wordpress-develop/assets/1682452/7b746e9b-b32b-4c06-81e8-131191fe36bf

#13 @afercia
3 months ago

@costdev yes I just tested the PR and it solves the a11y issue. I left a comment about an unrelated layout issue.

@costdev commented on PR #6078:


3 months ago
#14

@afercia Nicely spotted! I'll create a new ticket for this.

In the meantime, I've done a little digging into the overlap issue and to pave the way for a fix, I've moved the .plugin-dependencies notice DIV outside of the .column-description DIV. It's not part of the description anyway so this was a worthwhile change while we're here.

Can you give the PR another quick test to make sure everything's still working okay for the DOM order?

#15 @costdev
3 months ago

@huzaifaalmesbah Good idea about keeping the notice to the bottom of the card for consistency!

Let's address this in #60501.

#16 @bosskhj
3 months ago

Test Report

Description

When I apply the patch, it works perfectly.

Patch tested: https://github.com/WordPress/wordpress-develop/pull/6078

Environment

  • WordPress: 6.5-alpha-56966-src
  • PHP: 8.2.15
  • Server: nginx/1.25.3
  • Database: mysqli (Server: 8.0.36 / Client: mysqlnd 8.2.15)
  • Browser: Chrome 121.0.0.0
  • OS: macOS
  • Theme: Twenty Twenty-Four 1.0
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.1.0

Actual Results

  1. ✅ Issue resolved with patch.

Screenshot after patch apply

https://i.ibb.co/kDjDKkK/Screenshot-2024-02-13-at-12-42-25-AM.png

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


3 months ago

@afercia commented on PR #6078:


3 months ago
#18

I just re-tested this PR and I can confirm it solves the order issue.
Besides the overlapping problem mentioned in a previous comment at https://github.com/WordPress/wordpress-develop/pull/6078#issuecomment-1938337017 that should be addressed separately, this looks good to me @costdev thank you ❤️

#19 @costdev
3 months ago

  • Keywords commit added; reporter-feedback removed

PR 6078 has approval from Andrea.

The overlapping issue will be resolved in #60501.

Prepping for commit now.

#20 @costdev
3 months ago

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

In 57679:

Plugins: Output plugin card elements in the order they're displayed.

Previously, the notice for listing dependencies in a plugin card was styled with the CSS order properties. This created a mismatch between the visual order and DOM order of elements in the plugin card.

For accessibility, visual order and DOM order must always match when they affect meaning and functionality.

This removes the CSS order properties and outputs the dependencies notice later, making the visual and DOM order match. Some unused/empty CSS is also removed.

Follow-up to [57545].

Props afercia, afragen, bosskhj, huzaifaalmesbah, mukesh27, costdev.
Fixes #60488.

@costdev commented on PR #6078:


3 months ago
#21

Committed in r57679.

Note: See TracTickets for help on using tickets.