WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.52k stars 4.2k forks source link

createNotice shows "[object Object]" for createElement( RawHTML ) #17400

Open jsmoriss opened 5 years ago

jsmoriss commented 5 years ago

The following used to work, but now shows "[object Object]" instead of showing the desired HTML message.

Example:

var noticeHtml = '<p>Some HTML.</p>';

var noticeElement = createElement( RawHTML, {}, noticeHtml );

noticeObj = createNotice( noticeStatus, noticeElement, noticeOptions );

js.

jsmoriss commented 5 years ago

FYI - Here's a more complete example value for "noticeHtml". ;-)

<div class="wpsso-notice notice notice-alt notice-error error" id="err-post-1086-notice-missing-og-image" style="display:block;">
<div class="notice-label">SSO Notice</div>
<div class="notice-message">Text.</div>
</div>

js.

jsmoriss commented 5 years ago

This appears to be the problem:

packages/notices/src/store/actions.js

        // The supported value shape of content is currently limited to plain text
        // strings. To avoid setting expectation that e.g. a WPElement could be
        // supported, cast to a string.
        content = String( content );

There's no reason not to allow RawHTML as a WPElement.

js.

swissspidy commented 5 years ago

There's no reason not to allow RawHTML as a WPElement.

The danger in allowing that is developers are going to abuse it, especially when they can enter arbitrary HTML.

jsmoriss commented 5 years ago

There's no reason not to allow RawHTML as a WPElement.

The danger in allowing that is developers are going to abuse it, especially when they can enter arbitrary HTML.

Developers can abuse anything, if they put their mind to it. This is not a reason to limit other developers that want to use something correctly. And if developers abuse something in a way that users don't like, then users give negative feedback, don't buy the product, etc.

js.

jsmoriss commented 5 years ago

This kind of output is really annoying - is there nothing that can be done?

image

js.

jsmoriss commented 5 years ago

I would like to include a list of possible solutions, and links to resources for those solutions (settings pages and FAQs, for example), in error / warning notices.

How can I do this if Gutenberg typecasts the content to a (non-html) string?

createNotice() used to handle createElement( RawHTML ) just fine before someone added that typecast, which seems completely pointless when reading the packages/notices/src/store/actions.js comment:

        // The supported value shape of content is currently limited to plain text
        // strings. To avoid setting expectation that e.g. a WPElement could be
        // supported, cast to a string.
        content = String( content );

To avoid setting expectation that e.g. a WPElement could be supported, cast to a string.

I don't understand that comment. WPElement used to work just fine, and would work just fine if that typecast wasn't there. So there's no "could be supported" - no support is required. There seems to be more to this typecast than is explained in this comment. It sounds like someone decided that HTML in the content was a bad thing, which is really odd, since HTML is the foundation of a webpage experience. We create HTML to help users navigate content. Forcing warnings and errors to be text only, without links and proper forming (like <li> elements, for example), is detrimental to the user experience. How are we supposed to say "go here for more info"? Are we supposed to include a text URL and ask the user to copy-paste a text URL???

js.