Skip to content

Conversation

@ellatrix
Copy link
Member

Description

This branch fixes a few related issues:

  • The issue described at Format API #10209 (comment).
  • Inserting a non breaking zero-width space in the value is not a good thing, as it will break e.g. a search for text.
  • The non breaking zero-width space is kept around longer than it should.

The only solution for applying a format placeholder is to temporarily store an extra property on the value object that indicates to the editable tree creator that a format placeholder should appear there with the caret.

I added an e2e test to verify it works.

How has this been tested?

Ensure you can type while using formatting shortcuts (see e2e test). Ensure #10209 (comment) is not reproducible.

Screenshots

Types of changes

Bug fix.

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 the [Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable label Oct 26, 2018
@ellatrix ellatrix added this to the 4.2 milestone Oct 26, 2018
@ellatrix ellatrix requested review from a team and gziolo October 26, 2018 10:53
}
}

if ( isEditableTree && formatPlaceholder && formatPlaceholder.index === i + 1 ) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Logic needs to be unduplicated.

@youknowriad
Copy link
Contributor

This doesn't appear to fix the issue for me:

  • Type a word
  • Put the caret in the middle of the word
  • Hit the bold button two times
  • Select the whole word
  • Try applying bold
  • Notice the selection changes and the format is not applied properly.

@ellatrix ellatrix added the [Status] In Progress Tracking issues with work in progress label Oct 29, 2018
@ellatrix ellatrix removed request for a team and gziolo October 29, 2018 12:40
@ellatrix ellatrix force-pushed the try/rich-text-format-placeholder branch from d4f397c to 0993a6b Compare October 30, 2018 13:04
@ellatrix
Copy link
Member Author

@youknowriad I can't reproduce the bug. :/

@youknowriad
Copy link
Contributor

Not sure what happened in my initial testing, this does fix the issue indeed.

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@gziolo
Copy link
Member

gziolo commented Oct 30, 2018

I can confirm that the original issue was fixed. I have two more issues to report. I will do it separately.

@gziolo gziolo added [Type] Bug An existing feature does not function as intended and removed [Status] In Progress Tracking issues with work in progress labels Oct 30, 2018
@ellatrix ellatrix merged commit 393f5ba into master Oct 30, 2018
@ellatrix ellatrix deleted the try/rich-text-format-placeholder branch October 30, 2018 16:51
daniloercoli added a commit that referenced this pull request Oct 31, 2018
…rnmobile/port-quote-block-step-1

* 'master' of https://github.com/WordPress/gutenberg: (21 commits)
  Fix property path on get() call (#10962)
  Fixed typos on block api documentation (#11298)
  Export `switchToBlockType` to be used mobile side when merging two blocks. (#11294)
  RichText: Remove unused `ref` assignment to RichText (#11222)
  Remove findDOMNode from Tooltip component (#11169)
  Components: Remove redundant onClickOutside handler from Dropdown (#11253)
  added myself to the contributors list (#11260)
  Add complete post type labels for Resuable Blocks (#11278)
  Increase specificity for active radio/checkbox input styling (#11290)
  Fixed "artifact" misspelling in docs. (#11291)
  Nux package: fix incorrect named deprecated import (#11283)
  Rename parentClientId to rootClientId for consistency (#11274)
  chore(release): update changelog files
  chore(release): publish
  Update plugin version to 4.2.0. (#11258)
  Data: Use turbo-combine-reducers in place of Redux (#11255)
  Revert using Icon in IconButton to avoid regression in plugin icons (pinned icons) (#11256)
  Block List: Use default Inserter for sibling insertion (#11018)
  Editor: Optimize Inserter props generation and reconciliation (#11243)
  RichText: fix format placeholder (#11102)
  ...

# Conflicts:
#	packages/block-library/src/quote/index.js
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 [Type] Bug An existing feature does not function as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants