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

RichText: Ensure multiline prop is either "p" or "li" #10664

Merged
merged 2 commits into from
Oct 17, 2018

Conversation

ellatrix
Copy link
Member

@ellatrix ellatrix commented Oct 16, 2018

Description

Addresses: #10323 (comment) by @mcsf.

This change would ensure that the multiline prop on RichText is either p or li so block authors do not pass anything strange here. If they don't pass anything it will be set to p. The documentation suggests that p should be passed, I would change that to suggest just passing multiline for paragraphs. List is a bit of a special case here which we don't really want to put in the documentation, and may be deprecated in the future.

How has this been tested?

Ensure multiline instances work correctly. Test should catch any problems.

Screenshots

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.

@ellatrix ellatrix added this to the 4.0 milestone Oct 16, 2018
@ellatrix ellatrix requested a review from a team October 16, 2018 20:47
@ellatrix ellatrix added [Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable Backwards Compatibility Issues or PRs that impact backwards compatability labels Oct 16, 2018
Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested it with the following blocks:

  • List
  • Quote
  • Pullquote

Everything works perfectly fine. Code looks good, too. In the long run, we probably should leave multiline boolean and add some separate prop dedicated to lists. Unless we convert List block to work as a nested block and solve the issue this way :)

@gziolo gziolo merged commit 093e6fd into master Oct 17, 2018
@gziolo gziolo deleted the try/rich-text-multiline-restrict branch October 17, 2018 08:19
mcsf pushed a commit that referenced this pull request Nov 1, 2018
* Ensure multiline prop is either p or li

* Make booleans work
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 [Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants