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 🧹 #16536

Closed
wants to merge 9 commits into from
Closed

RichText 🧹 #16536

wants to merge 9 commits into from

Conversation

ellatrix
Copy link
Member

@ellatrix ellatrix commented Jul 11, 2019

Description

This PR mostly moves code around.

  • Move list controls to the list block, instead of embedding it in RichText.
  • Move Autocomplete to the RichText wrapper with block context.
  • Properly convert the multiline prop to a multilineTag.
  • Use the rich text value to determine when to merge. This removes the dependency on the DOM package.
  • Split onKeyDown in multiple functions.
  • Reduce code indentation (in one occasion 5 → 2 tabs).
  • Use hooks for RichText with block context.

How has this been tested?

E2e tests should pass.

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.
  • I've included developer documentation if appropriate.

@ellatrix ellatrix added [Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable [Type] Code Quality Issues or PRs that relate to code quality [Package] Rich text /packages/rich-text labels Jul 11, 2019
@ellatrix ellatrix added this to the Gutenberg 6.2 milestone Jul 11, 2019
@gziolo gziolo removed this from the Gutenberg 6.2 milestone Jul 30, 2019
Copy link
Contributor

@draganescu draganescu left a comment

Choose a reason for hiding this comment

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

I have tested these changes and found no issues, however there is too much here for me to simply approve. The gist is, things still work as expected after applying these changes.

@ellatrix
Copy link
Member Author

ellatrix commented Aug 5, 2019

Moved to #16905 because I renamed the branch to exclude the emoji.

@ellatrix ellatrix closed this Aug 5, 2019
@ellatrix ellatrix deleted the try/rich-text-🧹 branch August 5, 2019 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable [Package] Rich text /packages/rich-text [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants