Make WordPress Core

Opened 2 months ago

Closed 7 weeks ago

#60501 closed defect (bug) (fixed)

Plugin card dependencies notice and image can overlap.

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

Description

Comments on #60488 and its PR raised an issue and a change request:

  • Issue: @afercia When a plugin's name and description are short, the dependencies notice can partially appear behind the plugin's image.
  • Request: @huzaifaalmesbah The plugin dependencies notice should be weighted to the bottom of the .plugin-card-top DIV.

This ticket will address both of these. I'll be submitting a PR after the one for #60488 has been committed.

Attachments (5)

dependencies-notice-can-appear-behind-the-image.png (177.5 KB) - added by costdev 2 months ago.
Example of the dependencies notice appearing behind the plugin's image when the plugin's name and description are short.
60501.diff (971 bytes) - added by euthelup 7 weeks ago.
Move the plugins dependencies into its own box and snap it to the bottom.
60501-2.diff (1.3 KB) - added by euthelup 7 weeks ago.
Add equal spacing on all sides of plugin dependencies box. Also, avoid adding an empty plugin-card-center when there are no dependencies.
60501-3.diff (1.6 KB) - added by euthelup 7 weeks ago.
Alternative patch: Make use of notice classes and use $dependencies_notice directly without a wrapper.
Capture d’écran 2024-03-05 à 09.59.27.png (623.7 KB) - added by audrasjb 7 weeks ago.
After patch

Download all attachments as: .zip

Change History (20)

@costdev
2 months ago

Example of the dependencies notice appearing behind the plugin's image when the plugin's name and description are short.

#1 @costdev
2 months ago

  • Owner set to costdev
  • Status changed from new to assigned

@euthelup
7 weeks ago

Move the plugins dependencies into its own box and snap it to the bottom.

#2 @euthelup
7 weeks ago

I propose a simple fix: move the dependencies box from the plugin-card-bottom element into its own that can be pushed to the bottom with margin-top: auto. This way it won't overlap with the plugin logo and it will also snap to the bottom.

I have second thoughts about .plugin-card-center being semantically correct since it snaps to the bottom, but from what I've tested on macOS, Chrome, and Firefox, it works.

#3 @shailu25
7 weeks ago

Patch Tested: https://core.trac.wordpress.org/attachment/ticket/60501/60501.diff

Environment:

WordPress - 6.5-beta3
OS - Windows
Browser - Chrome
Theme: Twenty Twenty-Four
PHP - 8.1.23
Active Plugin - Test Reports

Screenshots:

Before Patch: https://prnt.sc/c1KvU8zauEHE
After Patch: https://prnt.sc/xmcT6GJ65EQX

Actual Results:

  • Dependencies notice Overlap with plugin's image issue is Resolved with patch.
  • Plugin dependencies notice is showing in the bottom with patch.

Suggestion: Can we add Equal space in bottom, same as Left & Right like this?
Ref: https://prnt.sc/UVsFdcq2w7vy

Last edited 7 weeks ago by shailu25 (previous) (diff)

@euthelup
7 weeks ago

Add equal spacing on all sides of plugin dependencies box. Also, avoid adding an empty plugin-card-center when there are no dependencies.

#4 @euthelup
7 weeks ago

Great catch.

I've addressed the spacing issue in the second patch.

#5 @shailu25
7 weeks ago

  • Keywords has-patch added; needs-patch removed

#6 @shailu25
7 weeks ago

@euthelup Thanks for the Patch Update.

60501-2.diff addressed both points as mentioned.

  • Dependencies notice Overlap with plugin's image issue is Resolved with patch.✅
  • Plugin dependencies notice is showing in the bottom with patch. ✅

Screenshots:

Before Patch: https://prnt.sc/c1KvU8zauEHE
After Patch : https://prnt.sc/-Ql3-9JITjZ5

#7 @desrosj
7 weeks ago

It looks like 60501-2.diff does fix this issue. I don't love having to add a different class for this. I could have sworn there was something similar already, but haven't been able to find it.

Something else that came to mind looking at this. Is there a reason the standard admin notice classes would not work for this? That would avoid having to include specific styles for this one of notice class.

#8 @desrosj
7 weeks ago

One small note for future testing as well. Please upload all screenshots from testing directly to the ticket in Trac instead of linking off to external locations. It's very common for these links to die off making it impossible to know what the screenshot showed in the future should someone need to review the ticket at some point.

#9 follow-up: @swissspidy
7 weeks ago

Instead of adding a new CSS class, can't we just style the existing .plugin-dependencies class?

.plugin-card .plugin-dependencies {
    background-color: #e5f5fa;
    border-left: 3px solid #72aee6;
    padding: 15px;
    margin: auto 20px 20px;
}

Though I have to say the class name plugin-dependencies is very generic.

So +1 to reusing existing admin notice classes & styling where possible.

#10 in reply to: ↑ 9 @euthelup
7 weeks ago

Replying to desrosj:

It looks like 60501-2.diff does fix this issue. I don't love having to add a different class for this. I could have sworn there was something similar already, but haven't been able to find it.

I think that popular CSS frameworks use classes like pull-down or pull-bottom for features like this but I've never seen one in WordPress.

Something else that came to mind looking at this. Is there a reason the standard admin notice classes would not work for this? That would avoid having to include specific styles for this one of notice class.

Just to be sure, you are referring to .notice .notice-alt .notice-info right?

Replying to swissspidy:

Instead of adding a new CSS class, can't we just style the existing .plugin-dependencies class?

.plugin-card .plugin-dependencies {
    background-color: #e5f5fa;
    border-left: 3px solid #72aee6;
    padding: 15px;
    margin: auto 20px 20px;
}

Though I have to say the class name plugin-dependencies is very generic.

So +1 to reusing existing admin notice classes & styling where possible.

TBH I've added plugin-card-center just for the sake of consistency with the other two elements on the same level plugin-card-top and plugin-card-bottom.
If you all feel comfortable with only having plugin-dependencies div there I'm ok with it.

@euthelup
7 weeks ago

Alternative patch: Make use of notice classes and use $dependencies_notice directly without a wrapper.

#11 @costdev
7 weeks ago

  • Keywords needs-testing added

With 6.5 RC1 today, and this ticket being a defect (bug) ticket for something introduced during this cycle, this can remain in the 6.5 milestone for resolution during the RC period.

Adding needs-testing to ensure that 60501-3.diff makes use of the existing notice classes without a visual regression.

#12 @audrasjb
7 weeks ago

  • Keywords has-screenshots added; needs-testing removed

I can confirm 60501-3.diff fixes the issue (see above screenshot).
And the code looks better to me.

#13 @audrasjb
7 weeks ago

  • Keywords commit added

I think we can ship it before RC1. But this ticket can of course stay a bit longer in the milestone since it's a bug introduced in 6.5 and there is no translatable string change.

#14 @audrasjb
7 weeks ago

  • Owner changed from costdev to audrasjb

Self assigning for commit as Colin is not fully available right now.

#15 @audrasjb
7 weeks ago

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

In 57776:

Plugins: Fix an overlap issue with plugin cards dependencies notice.

This changeset fixes an issue where plugin card dependencies notice and plugin icon were overlapping in some cases.

Props costdev, euthelup, shailu25, desrosj, swissspidy, audrasjb.
Fixes #60501.

Note: See TracTickets for help on using tickets.