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 🧹 (clean up, move Autocomplete, remove DOM dependency) #16905

Merged
merged 8 commits into from Aug 14, 2019

Conversation

@ellatrix
Copy link
Member

commented Aug 5, 2019

I renamed this branch because @frontdevde had problems pulling Gutenberg because of the emoji in the branch name.

Description

This PR mostly moves code around.

  • 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).

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 this to the Gutenberg 6.3 milestone Aug 5, 2019
@ellatrix ellatrix referenced this pull request Aug 5, 2019
5 of 5 tasks complete
@ellatrix ellatrix changed the title RichText 🧹 RichText 🧹 (use hooks, split list logic, remove DOM dependency) Aug 7, 2019
@ellatrix ellatrix referenced this pull request Aug 7, 2019
0 of 5 tasks complete
@ellatrix ellatrix force-pushed the try/rich-text-cleanup branch 3 times, most recently from 1f16b2f to 34cdf95 Aug 7, 2019
@ellatrix ellatrix force-pushed the try/rich-text-cleanup branch from 34cdf95 to a412bb3 Aug 8, 2019
@ellatrix ellatrix changed the title RichText 🧹 (use hooks, split list logic, remove DOM dependency) RichText 🧹 (clean up, move Autocomplete, remove DOM dependency) Aug 8, 2019
@gziolo gziolo removed this from the Gutenberg 6.3 milestone Aug 12, 2019
@gziolo
gziolo approved these changes Aug 14, 2019
Copy link
Member

left a comment

This PR still needs to be rebased after some changes applied to master.

I tested a few blocks including Paragraph, Galler and List toghether with the autocompletion and everything works as expected.

I'm not fully skilled to process all changes around new onKeyDown implementation in the rich-text/src/component/index.js so you might want to ask someone else to focus on that part.

@ellatrix ellatrix force-pushed the try/rich-text-cleanup branch from a412bb3 to 5c6d75c Aug 14, 2019
@ellatrix ellatrix force-pushed the try/rich-text-cleanup branch from 5c6d75c to 9e4562a Aug 14, 2019
@ellatrix

This comment has been minimized.

Copy link
Member Author

commented Aug 14, 2019

I'm not fully skilled to process all changes around new onKeyDown implementation in the rich-text/src/component/index.js so you might want to ask someone else to focus on that part.-

I'm not sure who else would be more skilled. I merely moved some code around and sorted it by keyCode. The only change I made that is not just moving code around is swapping ! isHorizontalEdge, which uses the DOM, with

activeFormats.length ||
( isReverse && start !== 0 ) ||
( ! isReverse && end !== text.length )

so that we use the internal rich text value instead of the DOM to determine when to merge/delete.

It's the same thing.

Since we're at the start of a release cycle, I'll merge this now, so we can continue work on top of this, and there is enough time to catch problems if any appear.

@ellatrix ellatrix merged commit 24caaa3 into master Aug 14, 2019
4 checks passed
4 checks passed
Filter opened
Details
Filter opened
Details
Milestone It
Details
Travis CI - Pull Request Build Passed
Details
@ellatrix ellatrix deleted the try/rich-text-cleanup branch Aug 15, 2019
@ellatrix ellatrix added this to the Gutenberg 6.4 milestone Aug 15, 2019
gziolo added a commit that referenced this pull request Aug 29, 2019
* RichText 🧹

* Clean up

* Clean up

* Simplify

* Move Autocomplete

* children => Editable wrapper component

* Bind Editable wrapper to class

* Minimise diff
gziolo added a commit that referenced this pull request Aug 29, 2019
* RichText 🧹

* Clean up

* Clean up

* Simplify

* Move Autocomplete

* children => Editable wrapper component

* Bind Editable wrapper to class

* Minimise diff
dd32 pushed a commit to dd32/gutenberg that referenced this pull request Sep 27, 2019
…Press#16905)

* RichText 🧹

* Clean up

* Clean up

* Simplify

* Move Autocomplete

* children => Editable wrapper component

* Bind Editable wrapper to class

* Minimise diff
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.