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

Avoid change in RichText when possible #6872

Merged
merged 1 commit into from May 22, 2018

Conversation

Projects
None yet
3 participants
@iseulde
Member

iseulde commented May 21, 2018

Description

This is #6455 (which has been reverted in #6455) plus a fix to ensure formatting changes are synced, and a history level is added. We are using the formatter directly which doesn't trigger an execCommand event as it does with core TinyMCE buttons. Those buttons make use of the mceToggleFormat command. This can be fix by calling onCreateUndoLevel directly in the formatting functions we have.

The reason this works in master and stopped working with #6455 is that the editor is blurred and focussed whenever formatting is applied (except for the first time).

How has this been tested?

See #6455. Also ensure a history record is created (and the update button enabled) when you make something bold as the first change you make. This last issue is broken in master too.

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.

@iseulde iseulde requested review from WordPress/gutenberg-core and youknowriad May 21, 2018

@iseulde iseulde added this to the 3.0 milestone May 21, 2018

@youknowriad

This comment has been minimized.

Contributor

youknowriad commented May 22, 2018

I'm seeing a regression here, when I click on the "link" control the modal doesn't show up, I have to click twice to make it work. I wonder why it's not being catched by the e2e tests since we have tests for the link modal.

@youknowriad

This comment has been minimized.

Contributor

youknowriad commented May 22, 2018

Actually, after refreshing the page it's working properly. I guess we have a bug somewhere else then.

@youknowriad

LGTM 👍

@iseulde

This comment has been minimized.

Member

iseulde commented May 22, 2018

Awesome! Fingers crossed it doesn't break anything this time. :)

@iseulde iseulde merged commit 3f4335d into master May 22, 2018

2 checks passed

codecov/project 46.42% (-0.03%) compared to ce61b59
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@iseulde iseulde deleted the try/rich-text-avoid-change branch May 22, 2018

@aduth

This comment has been minimized.

Member

aduth commented May 23, 2018

Could end-to-end tests help / have helped avoid regressions?

@youknowriad

This comment has been minimized.

Contributor

youknowriad commented May 23, 2018

@aduth End to end tests to avoid the regressions introduced by the previous PR have been added in #6865

@aduth

This comment has been minimized.

Member

aduth commented May 23, 2018

Perfect, thanks @youknowriad 👍

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