Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom classes and IDs are removed from admin notices #12320

Closed
desrosj opened this issue Nov 26, 2018 · 3 comments
Closed

Custom classes and IDs are removed from admin notices #12320

desrosj opened this issue Nov 26, 2018 · 3 comments
Labels
Backwards Compatibility Issues or PRs that impact backwards compatability [Package] Notices /packages/notices [Type] Bug An existing feature does not function as intended

Comments

@desrosj
Copy link
Contributor

desrosj commented Nov 26, 2018

Companion ticket for Trac-45418.

Custom classes and IDs are stripped from admin notices, making it impossible for plugin and theme developers to specifically target their notices.

Describe the bug
A clear and concise description of what the bug is.

To Reproduce
Add an admin notice using the admin_notices action hook that has custom classes and an ID attribute.

add_action( 'admin_notices', function() {
	?>
	<div id="my-plugin-error-notice" class="notice notice-error my-plugins-custom-class"><p>This is a custom error notice for my plugin.</p></div>
	<?php
});

Open the editor screen and inspect the notice. It has the Gutenberg notice class (components-notice is-error), and nothing else.

Expected behavior
The custom classes and ID should be preserved.

@desrosj desrosj added [Type] Bug An existing feature does not function as intended Backwards Compatibility Issues or PRs that impact backwards compatability [Package] Notices /packages/notices labels Nov 26, 2018
@chrisvanpatten
Copy link
Contributor

I think this was an intentional change, although there are definitely back-compat implications. I think the long term idea is for core to limit the amount of customisation to notices, so we can build out a better system for managing and displaying notifications. @aduth can probably add more context here :)

@aduth
Copy link
Member

aduth commented Nov 26, 2018

This is a consequence of the notices module being more constrained in its understanding of a "notice" entity, and thus prone to lossy conversion in the upgrade for admin_notices backwards-compatibility (#11604).

An interim workaround can be to leverage the fact that the inner markup of a notice is preserved verbatim, and thus it's possible to include a wrapping element within the .notice upon which an ID is applied.

add_action( 'admin_notices', function() {
	?><div class="notice notice-error"><div id="my-plugin-error-notice" class="my-plugins-custom-class"><p>This is a custom error notice for my plugin.</p></div></div><?php
} );

As to a general solution? To the use-case of Trac-45418, ideally the dismiss remembrance is something which could be baked into the default notices behavior. Other ideas to explore could include:

  • Detect whether there are attributes or classes assigned to the admin_notices notice and, if present, create some wrapping element automatically
  • Enhance notices module to support "attributes" broadly, and pass through all attributes of the admin_notices notice

These should be tempered by an understanding that the implementation of preserving markup in #11604 via __unstableHTML is intended to serve as a stop-gap solution.

@desrosj
Copy link
Contributor Author

desrosj commented Dec 3, 2018

I am going to close this. Legacy admin notices are no longer being consumed and displayed by Gutenberg as of #12444.

@desrosj desrosj closed this as completed Dec 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backwards Compatibility Issues or PRs that impact backwards compatability [Package] Notices /packages/notices [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

3 participants