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.

Sign up for GitHub

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

Jump to bottom

Framework: split block logic out of RichText (part 2) #16309

Merged
merged 10 commits into from
Jul 10, 2019
Merged

Framework: split block logic out of RichText (part 2) #16309

merged 10 commits into from
Jul 10, 2019

Conversation

ellatrix
Copy link
Member

@ellatrix ellatrix commented Jun 26, 2019 •
edited
Loading

Description

This PR splits block related logic out of RichText into the RichText wrapper for blocks.

The RichText wrapper (to be used in blocks) now implements:

  • onEnter
  • onDelete (triggered at edges)
  • onPaste
  • inputRule ( for * to list etc.)

Also moves format relate logic to the format library:

  • Backtick ` rule to transform to <code>.
  • A paste rule for to link works on pasting a URL.
    • Also fixes the link pop up not showing after pasting the link.

How has this been tested?

E2e tests. Everything should work as before.

Screenshots

Types of changes

Refactoring.

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 [Status] In Progress Tracking issues with work in progress [Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable labels Jun 26, 2019
@ellatrix ellatrix self-assigned this Jun 26, 2019
@ellatrix ellatrix force-pushed the try/rich-text-split-2 branch from 26f1136 to f8d5774 Compare June 26, 2019 17:42
@ellatrix ellatrix force-pushed the try/rich-text-split-2 branch from a289478 to f2342e5 Compare June 27, 2019 10:19
@ellatrix ellatrix added [Type] Code Quality Issues or PRs that relate to code quality and removed [Status] In Progress Tracking issues with work in progress labels Jun 27, 2019
@ellatrix ellatrix changed the title RichText: split block logic (part 2) Framework: split block logic out of RichText (part 2) Jun 27, 2019
@ellatrix ellatrix added the Framework Issues related to broader framework topics, especially as it relates to javascript label Jun 27, 2019
@ellatrix ellatrix added this to the Gutenberg 6.1 milestone Jun 27, 2019
@ellatrix ellatrix force-pushed the try/rich-text-split-2 branch from beeeaf8 to 92532b2 Compare June 27, 2019 16:25
jorgefilipecosta
jorgefilipecosta approved these changes Jul 9, 2019
View reviewed changes
Copy link
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

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

Awesome work here @ellatrix, I did not find any regressions, and these changes seem to put the API in a better direction.
I left some comments but I don't think there are blockers.

packages/block-editor/src/components/rich-text/index.js
let adjustedOnChange = originalOnChange;

// Handle deprecated format.
if ( Array.isArray( originalValue ) ) {
Copy link
Member

Choose a reason for hiding this comment

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

Should we add a deprecated call, to warn developers that a deprecated format is being used?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. I believe a warning is logged from the blocks api package when this type of source is used, but I will test again to confirm.

Copy link
Member Author

Choose a reason for hiding this comment

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

As far as I can see, the deprecation were removed and never reintroduced. In other words, what @youknowriad suggested has never been done: #10439 (comment). Let's do this in a separate PR.

@ellatrix ellatrix merged commit 03121c5 into master Jul 10, 2019
@ellatrix ellatrix deleted the try/rich-text-split-2 branch July 10, 2019 05:48
@ellatrix
Copy link
Member Author

Thanks for the review @jorgefilipecosta! :)

@ellatrix
Copy link
Member Author

The base RichText is again under a 1000 loc! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jorgefilipecosta jorgefilipecosta jorgefilipecosta approved these changes

@aduth aduth Awaiting requested review from aduth

@daniloercoli daniloercoli Awaiting requested review from daniloercoli

@etoledom etoledom Awaiting requested review from etoledom

@SergioEstevao SergioEstevao Awaiting requested review from SergioEstevao

@youknowriad youknowriad Awaiting requested review from youknowriad

Assignees

@ellatrix ellatrix

Labels
[Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable Framework Issues related to broader framework topics, especially as it relates to javascript [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Milestone
Gutenberg 6.2
Development

Successfully merging this pull request may close these issues.
3 participants
@ellatrix @jorgefilipecosta @noisysocks