WordPress.org

Make WordPress Core

Opened 9 months ago

Closed 8 months ago

Last modified 5 months ago

#46275 closed defect (bug) (invalid)

Use CSS not paragraph tag for plugin install card notice

Reported by: afragen Owned by:
Milestone: 5.2 Priority: normal
Severity: normal Version: 5.1
Component: Site Health Keywords: servehappy has-patch
Focuses: Cc:
PR Number:

Description

In r43436 we added a paragraph tag to the plugin card for the error notice. Given some of the issues we are having in #46044 I believe a better solution is to remove the paragraph tag and target the CSS. This should mitigate at least one of the issues we have.

Attachments (3)

46275.diff (1.5 KB) - added by afragen 9 months ago.
46275.2.diff (1.5 KB) - added by afragen 9 months ago.
fixed for when annotation not present
46275.3.diff (3.2 KB) - added by afragen 8 months ago.
also fix in install plugin iframe

Download all attachments as: .zip

Change History (13)

@afragen
9 months ago

@afragen
9 months ago

fixed for when annotation not present

#1 @pento
9 months ago

  • Milestone changed from Awaiting Review to 5.2

@afragen
8 months ago

also fix in install plugin iframe

This ticket was mentioned in Slack in #core-php by afragen. View the logs.


8 months ago

#3 @spacedmonkey
8 months ago

@afragen Can you explain in a little more detail why this ticket is important and why it is needed. It seems unrelated to servehappy and a styling fix. Not to say it shoudn't be merged at some point.

#4 @afragen
8 months ago

There are several locations where paragraph tags are used for spacing purposes that can be more efficiently done with CSS. This patch also removes several surrounding paragraph tags that were inserted for earlier Servehappy commits. https://core.trac.wordpress.org/ticket/43987#comment:36

Combined with the issue of having nested paragraph tags causing browsers to create additional paragraph tags pairs resulting in empty paragraph tag pairings.

As the default parameters for the wp_update_php_annotaation() are paragraph tags, this change obviates needing to place the <br><span>... parameters in about 4 to 5 other locations.

If I’m not explaining this well, please let me know.

#5 @TimothyBlynJacobs
8 months ago

Generally core <div class="notice"> components require inner <p> tags. If I'm reading the patch right, this would remove those from the plugin cards and make them unnecessary because they are spaced with CSS. That inconsistency doesn't seem great, but maybe isn't something to worry too much about?

#6 @afragen
8 months ago

The specific p tags that are removed here were added in a previous servehappy commit as referenced above. The thought was to avoid having multiple instances of needing to add parameters to wp_update_php_annotation().

It was a thought on my part. I can always update the other 46269 to pass those parameters. Adding a p tag didn’t seem to add semantic markup as much as it was being used for spacing.

#7 @desrosj
8 months ago

Instead of adding another CSS definition, can empty $before and $after parameters for wp_update_php_annotation() (pending #46044) be used to solve this issue instead?

#8 @afragen
8 months ago

I’ll need to test to see if that also solves the problem and looks similar.

#9 @afragen
8 months ago

  • Keywords dev-feedback removed
  • Resolution set to invalid
  • Status changed from new to closed

Fixed in a better fashion without CSS changes in #46269

Last edited 8 months ago by afragen (previous) (diff)

#10 @spacedmonkey
5 months ago

  • Component changed from Upgrade/Install to Site Health
Note: See TracTickets for help on using tickets.