Update message displays unstyled in plugins list

[ovann86]

Appears to have occurred after changeset r37714

https://core.trac.wordpress.org/changeset/37714/trunk/src/wp-admin/css/list-tables.css

When a plugin uses an update message, for example a registration reminder, some basic styling is applied to make it stand out - background red, padding etc.

In 4.6-RC1-38170 the styles aren't applied and are missing from /wp-admin/css/list-tables.css

[ovann86]

WordPress 4.5.x - Installed Plugins page with update message correctly styled

[ovann86]

WordPress 4.6-RC1-38170 - Installed Plugins page with update message unstyled

[ovann86]

Plugin to demonstrate a plugin update message

[afragen]

Shiny Updates has merely required some changes to the class tags that go into the after plugin row data. I think it may also require the plugin to wrap the message in a p tag also.

I'm not sure why the plugin dev is not responsible for updating their code. Isn't this exactly why we release betas?

This is how I did it in a plugin, ​https://github.com/afragen/github-updater/blob/develop/src/GitHub_Updater/Base.php#L1024-L1055

[Clorith]

I had a quick look, it's a simple process to re-introducing everything with the exception of the spinner icon.

Previously we attached the icon to .update-message:before, but we now target .update-message p:before instead.

Re-introduction with the spinner would mean that shiny updates receives two of them, so this might require some output changes to shiny updates output as well. Alternatively we could omit the icon and just attach the other styles so it doesn't look like something is broken?

Replying to afragen:

I'm not sure why the plugin dev is not responsible for updating their code. Isn't this exactly why we release betas?

It is, but there's quite a few plugins that utilizes this approach, and expecting them all to have sorted it is (unfortuantely) unrealistic, and having a somewhat broken UI isn't great for the user, and the user should always be our priority.

Normally if we introduce things that could break how things are perceived, we'd have included it in one of the posts leading up to the release though, but the CSS changes haven't been mentioned so many authors may be blissfully unaware of this change.

[rahulsprajapati]

Added css for WP < 4.6 version update message displays using :not(.notice) as .notice class is added after 4.6

[Clorith]

37504.patch sort of works, but there's a visual separation, you may want to have a quick look at list-tables.css line 1294 which covers removing box-shadows of the item that owns the update line, the attached screenshot covers the patched view first, then shiny updates.

Not sure how much it matters, but the box shadow does make it feel like there's meant to be plugin data that is missing inside that table row.

[rahulsprajapati]

Refreshed Patch : Fixed box-shadow as mention in comment:8 by @Clorith

[rahulsprajapati]

Replying to Clorith:

37504.patch sort of works, but there's a visual separation, you may want to have a quick look at list-tables.css line 1294 which covers removing box-shadows of the item that owns the update line, the attached screenshot covers the patched view first, then shiny updates.

Not sure how much it matters, but the box shadow does make it feel like there's meant to be plugin data that is missing inside that table row.

Please check 37504.1.patch.

[obenland]

Right, the parent tr needs an .update class to remove the box shadow, but that hasn't changed from 4.5.

I think the latest patch achieves what we're looking for in terms of backwards compatibility (although the box shadow bits can be removed). It still makes for a poor user experience in my opinion when core elements are hijacked like this. If there is an update to a plugin adding a custom notification it will result in two notifications being shown. And if it doesn't pretend to have an update available, the notice hangs in between two plugin rows, visually not belonging to either really, while sporting an update dashicon for a non-update message. See 37504.PNG​.

[Clorith]

Patch looks good for backwards compatibility, there's not much more we can (or should) do with this one.

Ideally those who use the update area for other messages should move them over to admin notices instead, and hopefully will after 4.6.

[DrewAPicture]

37504.1.patch​ looks good and works well for me.

[helen]

I'm not sure I understand why the :not(.notice) selectors are necessary. Also, since there are different notice styles, please test and make sure that the colors can still be overridden by those classes (e.g. .notice-info).

[rahulsprajapati]

@helen

I have used :not(.notice) selector because of this markup we are using in update message from 4.6

WP 4.6 :

<div class="update-message notice inline notice-warning notice-alt">
  <p>Update Message..</p>
</div>

WP < 4.6 :

<div class="update-message">
   Update Message..
</div>

Now, here I can see only one way for backwards compatibility, using :not(.notice) selector. I did check this .update-message class in the core and it's not used in other than update message sections. So I think .update-message:not(.notice) should not conflict with any other CSS of the core.
We can also use notice-warning class for :not selector.

Thank you. :)

[ocean90]

37504.2.patch​ is a refresh of 37504.1.patch​:

• Replace 6.5px with 0.5em
• One selector per line
• Merge the styles for .notice and .update-message
• Adds a /* back-compat for pre-4.6 */ comment
• Removes a stray comma

---

An alternative would be to replace all .update-message classes with a new class for the new styles and restore the old styles as .update-message

[ocean90]

37504.3.patch​:

• Removes the :not(.notice) selector from .plugins .plugin-update-tr .update-message:not(.notice). .plugins .plugin-update-tr .update-message is the same as .plugins .plugin-update-tr .notice.
• Removes .update-message:not(.notice) from the .plugins .notice p rule. The rule for.plugins .plugin-update-tr .update-message has already the correct margins.
• Merges .update-message:not(.notice):before with .wrap .notice p:before.

[ocean90]

Replying to helen:

I'm not sure I understand why the :not(.notice) selectors are necessary. Also, since there are different notice styles, please test and make sure that the colors can still be overridden by those classes (e.g. .notice-info).

HTML markup of a table row for an update in…

…WordPress 4.5:

<tr class="plugin-update-tr active" id="hello-dolly-update" data-slug="hello-dolly" data-plugin="hello.php">
        <td colspan="3" class="plugin-update colspanchange">
                <div class="update-message"> <!-- This element needs to be addressed. -->
                        There is a new version of Hello Dolly available. <a href="" class="thickbox open-plugin-details-modal" aria-label="View Hello Dolly version 1.6 details">View version 1.6 details</a> or <a href="" class="update-link" aria-label="Update Hello Dolly now">update now</a>.
                </div>
        </td>
</tr>

…trunk:

<tr class="plugin-update-tr" id="hello-dolly-update" data-slug="hello-dolly" data-plugin="hello-dolly/hello.php">
        <td colspan="3" class="plugin-update colspanchange">
                <div class="update-message notice inline notice-warning notice-alt">
                        <p>
                                There is a new version of Hello Dolly available. <a href="" class="thickbox open-plugin-details-modal" aria-label="View Hello Dolly version 1.6 details">View version 1.6 details</a> or <a href="" class="update-link" aria-label="Update Hello Dolly now">update now</a>.
                        </p>
                </div>
        </td>
</tr>

By using :not(.notice) we only address the element which has the .update-message class, no other notice classes are involved.
To make this clearer we could use another selector: div[class="update-message"], see 37504.4.patch​.

Proposing 37504.3.patch​ or 37504.4.patch​ for commit.

[afragen]

Just tossing my novice opinion that 37504.4 appears a cleaner solution.

Also, 4.5 plugin row messages won't have a spinner without the surrounding <p> tags but that's probably a good thing. Here I'm referring to a message as in @Clorith's first image above.

[SergeyBiryukov]

Replying to ocean90:

To make this clearer we could use another selector: div[class="update-message"], see 37504.4.patch​.

Any reason it's not just div.update-message?

[swissspidy]

@SergeyBiryukov Because that would address div.update-message.notice too, which is not wanted here.

[SergeyBiryukov]

Thanks, looks good to me then.

[ocean90]

Without icon: ​https://cloudup.com/cfmJHMcJLFA

[DrewAPicture]

@ocean90 What would be the purpose behind removing the icon? If the plugin isn't adding their own new row it's going to have the icon anyway.

My +1 is on 37504.4.patch​.

[afragen]

+1 for 37504.5.patch (without update icon)

Isn't this is ticket about adding an after_plugin_row_{$slug} for registration or other notices. It doesn't make sense to add the update icon.

@DrewAPicture if the plugin isn't adding their own new row there won't be a difference. If, however, the plugin is adding a new row. Say for a registration notice or anything other that an update notice, there's no reason to have the icon. At least that's how I use it.

[DrewAPicture]

@afragen See the linked discussion in Slack. I was making the assumption that developers using the update-message class on a div would be using it for update message purposes.

The other suggestion I made was to simply avoid it looking completely broken by only supporting back-compat on the padding and margin but not the colors/icon at all, i.e. ​https://cl.ly/1I460V452u3c

In that screenshot, the first one is the row added by the plugin. The second one is added by core.

[afragen]

@DrewAPicture sorry I saw the trac message before looking at Slack :P

Unfortunately using .update-message is the simplest way to add a similarly styled row here. I agree with you idea behind just setting the margin/padding so it doesn't appear broken. It should be on the dev implementing this to fix/style it how they choose. I would be quite happy with just having a margin/padding fix. For a non-update notice in this position the colors/border/icon aren't really needed, but I think the simplest fix here is probably the best.

What bothers me is the extra "border" line on the top of such a non-update notice giving it the appearance that it's somehow separate from the plugin above. This seems to be from a box-shadow but still would need the bottom of that, just not the top. BTW, this doesn't happen for actual update messages. Perhaps another ticket is needed for this.

[DrewAPicture]

w/ 37504.diff​

[DrewAPicture]

w/ 37504.diff​

[DrewAPicture]

w/ 37504.diff​

[DrewAPicture]

Added 37504.diff​, which restores the padding and margin only for the update-message class.

This approach is simple, and provides basic styling so it doesn't look as broken, and makes no assumptions about what the added div contains.

Edit: Note: The double notice in the screenshots is due to the plugin only displaying the extra row on subsites in multisite, which apparently were not supported at some point previously. The top one is the notice added by the plugin, the bottom supplied by core.

If the plugin is inactive, looks like 37504diff_inactive.png​.

If the plugin is active and the plugin hasn't incorporated the .active class alongside .plugin-update-tr on the <tr>, looks like 37504diff_active_no_class.png​.

If the plugin is active and the plugin has incorporated the .active class, looks like 37504_active_with_class.png​.

[afragen]

Added 37504.diff​, which restores the padding and margin only for the update-message class.

I like this.

[DrewAPicture]

In 38237:

Update/Install: Provide basic back-compat styling for the .update-message CSS class in the plugins list table.

This change restores only the margin and padding styles for the .update-message class when used by plugins in the context of adding arbitrary rows to the list table. The inline-update colors and icon styles were not restored, expressly with a wide variety of plugin use-cases in mind.

Props ovann86, rahulsprajapati, ocean90, DrewAPicture.
Props helen for review.
See #37504.

[DrewAPicture]

In 38238:

Update/Install: Provide basic back-compat styling for the .update-message CSS class in the plugins list table.

This change restores only the margin and padding styles for the .update-message class when used by plugins in the context of adding arbitrary rows to the list table. The inline-update colors and icon styles were not restored, expressly with a wide variety of plugin use-cases in mind.

Merge of [38237] to the 4.6 branch.

Props ovann86, rahulsprajapati, ocean90, DrewAPicture.
Props helen for review.
Fixes #37504.