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

Refactor remove line separator to method in the rich-text package. #15946

Merged
merged 5 commits into from
Jun 5, 2019

Conversation

SergioEstevao
Copy link
Contributor

Description

This PR refactor the logic for removing a line separator inside a list to use a function on the rich text library.

This allows both the native and the web part to share the same code and simplifies the code on the rich text editor component.

How has this been tested?

Screenshots

Types of changes

Refactoring of method.

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.

@SergioEstevao SergioEstevao added [Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable [Package] Rich text /packages/rich-text labels May 31, 2019
@SergioEstevao SergioEstevao added this to the 5.9 (Gutenberg) milestone May 31, 2019
Copy link
Contributor

@etoledom etoledom left a comment

Choose a reason for hiding this comment

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

Nice step to share more code between mobile and web RichText!

It works well on mobile ✅ But let's wait for @aduth or @ellatrix 's opinion too (or someone else's?), since this also modifies web side files.

A small detail: I think there's a typo in the docs comments. I added it as a code comment.

@aduth
Copy link
Member

aduth commented Jun 4, 2019

I can review, but @ellatrix would be in the best position to judge whether this change is a sensible addition to the rich-text API.

Copy link
Member

@ellatrix ellatrix left a comment

Choose a reason for hiding this comment

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

Sounds good. Good that it's marked unstable.

@SergioEstevao SergioEstevao merged commit 788a3d2 into master Jun 5, 2019
@SergioEstevao SergioEstevao deleted the rnmobile/refactor_remove_line_separator branch June 5, 2019 12:51
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants