WordPress.org

Make WordPress Core

Opened 15 months ago

Closed 14 months ago

Last modified 11 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:

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 15 months ago.
46275.2.diff (1.5 KB) - added by afragen 15 months ago.
fixed for when annotation not present
46275.3.diff (3.2 KB) - added by afragen 15 months ago.
also fix in install plugin iframe

Download all attachments as: .zip

Change History (13)

@afragen
15 months ago

@afragen
15 months ago

fixed for when annotation not present

#1 @pento
15 months ago

  • Milestone changed from Awaiting Review to 5.2

@afragen
15 months ago

also fix in install plugin iframe

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


15 months ago

#3 @spacedmonkey
15 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
15 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
14 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
14 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
14 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
14 months ago

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

#9 @afragen
14 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 14 months ago by afragen (previous) (diff)

#10 @spacedmonkey
11 months ago

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