Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#46275 closed defect (bug) (invalid)

Use CSS not paragraph tag for plugin install card notice

Reported by: afragen's profile 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 5 years ago.
46275.2.diff (1.5 KB) - added by afragen 5 years ago.
fixed for when annotation not present
46275.3.diff (3.2 KB) - added by afragen 5 years ago.
also fix in install plugin iframe

Download all attachments as: .zip

Change History (13)

@afragen
5 years ago

@afragen
5 years ago

fixed for when annotation not present

#1 @pento
5 years ago

  • Milestone changed from Awaiting Review to 5.2

@afragen
5 years ago

also fix in install plugin iframe

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


5 years ago

#3 @spacedmonkey
5 years 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
5 years 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
5 years 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
5 years 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
5 years 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
5 years ago

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

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

#10 @spacedmonkey
5 years ago

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