Opened 12 months ago
Closed 11 months ago
#60501 closed defect (bug) (fixed)
Plugin card dependencies notice and image can overlap.
Reported by: |
|
Owned by: |
|
---|---|---|---|
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)
Change History (20)
#2
@
12 months 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
@
12 months 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
@
12 months ago
Add equal spacing on all sides of plugin dependencies box. Also, avoid adding an empty plugin-card-center
when there are no dependencies.
#6
@
12 months 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
@
12 months 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
@
12 months 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:
↓ 10
@
12 months 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
@
12 months 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.
@
12 months ago
Alternative patch: Make use of notice
classes and use $dependencies_notice
directly without a wrapper.
#11
@
12 months 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
@
11 months 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
@
11 months 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.
Example of the dependencies notice appearing behind the plugin's image when the plugin's name and description are short.