Button Block: Properly render inner HTML if context-appropriate

[Copons]

Fixes #15578

Changes proposed in this Pull Request:

• If the Button block is set to output as a or button, make sure the inner HTML is rendered.
• If the Button block is set to output as input, prevent inserting newlines and strip any HTML tags.

Testing instructions:

• Insert a Revue block, and add a Revue username (or some random text).
• Smoke test the block CSS classes and styles: set custom colors, change the border radius. Make sure they are rendered as expected in the front end.
• Try inserting a newline in the button text.
• Make sure the newline appears in the front end, and there is no unparsed HTML content.

Now modify the Revue inner block definition.

• Change the element into an a, and add an url attribute (with an URL as value).
• Reload and insert a new Revue block, adding some newlines in the button text.
• Make sure the newline appears in the front end, and clicking the button opens the url in a new tab (also it should have the following attributes: target="_blank" role="button" rel="noopener noreferrer").

Modify the Revue inner block definition again.

• Change the element into an input, and add an uniqueId attribute (any value will do).
• Reload and insert a new Revue block, and try adding some newlines in the button text.
• Make sure the newlines are converted into simple spaces.
• Make sure there aren't any newlines in the front end as well, and check if the data-id-attr and id attributes are set to the uniqueId defined above.
• Note: ignore the fact that the button width is now the same as the other input fields. It's a Revue CSS setting, which don't expect the button to be an input element.

Proposed changelog entry for your changes:

• Button Block: Properly render inner HTML if context-appropriate

[jetpackbot]

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: May 5, 2020.
Scheduled code freeze: April 28, 2020

Generated by 🚫 dangerJS against 2343a54

[scruffian]

LGTM

[jeherve]

This is problematic in some cases. As an example, you may have used "enter" to create a bigger button:

And when we lose the line breaks, we lose the space between the words:

[matticbot]

Caution: This PR has changes that must be merged to WordPress.com
Hello Copons! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D42632-code before merging this PR. Thank you!
This revision will be updated with each commit to this PR

[Copons]

This is problematic in some cases. As an example, you may have used "enter" to create a bigger button:

And when we lose the line breaks, we lose the space between the words:

@jeherve That's exactly what the future disableLineBreaks prop will prevent. 🙂

Though, I didn't consider this kind of use case.
I've pushed a simple string replacement that strips <br> off the button content.
This seems to also work when pasting multiline strings.

Actually, it replaces <br> with a space.
Removing the it altogether would mean that when pressing enter, the field value effectively remains the same and setAttribute is not triggered, while the component will indeed render the newline regardless.
E.g.

• Initial content: Subscribe.
• Press enter. Field value: Subscribe<br>.
• Strip the line break: Subscribe.
• setAttribute( { text: 'Subscribe' } ) doesn't do anything.

I've marked the change as to be removed when Gutenberg 8.0 is our minimum supported version, which means that we can fully rely on disableLineBreaks.

[glendaviesnz]

This works for me. It is a slightly odd experience hitting enter or shift-enter and getting a space, but seemed to produce string with correct spacing in all instances, including copy/paste.

I am a bit late to the party on the button discussions, so apologies if this has already been covered, but would it be possible to only strip line breaks of the block element attribute is input, that at least in the cases when it isn't it would be possible to have line breaks for the use case that Jeremy showed?

[Copons]

This works for me. It is a slightly odd experience hitting enter or shift-enter and getting a space, but seemed to produce string with correct spacing in all instances, including copy/paste.

I am a bit late to the party on the button discussions, so apologies if this has already been covered, but would it be possible to only strip line breaks of the block element attribute is input, that at least in the cases when it isn't it would be possible to have line breaks for the use case that Jeremy showed?

Good point @glendaviesnz!
I've updated the PR with a more involved approach, and we now only strip HTML and prevent line breaks if the button is an input element.

Unfortunately, DOMDocument doesn't easily allow nesting HTML content.
There are workarounds, but all felt overkill here, where we have a simple enough set of variations.
I've chosen to change the implementation, and go with a simple string concatenation approach.

[jeherve]

This seems to test well for me. Merging.

In the future and as we'll probably use the Button block more and more, I think it would be nice to add some tests to cover the different functions in extensions/blocks/button/button.php. Maybe in a future PR?

[jeherve]

Cherry-picked to branch-8.5 in 544659c

[kraftbj]

r206786-wpcom