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

Upgrade Draftail to v1.2.1. Fix #4985, adds more Markdown shortcuts #5117

Merged

Conversation

Projects
None yet
2 participants
@thibaudcolas
Copy link
Member

thibaudcolas commented Mar 4, 2019

This includes:

Here is the combined CHANGELOG for what's relevant to Wagtail:

Bug fixes

  • #4985 – Prevent crash when filtering pasted content whose last block is to be removed (e.g. unsupported image) (#179).
  • Stop unnecessarily calling onSave in the editor’s onBlur (#173).
  • Prevent crash in DraftUtils.getEntitySelection, when the provided entity key isn't valid (undefined, missing) (#168).
  • Fix entity removal and editing not doing anything when the selection is backwards (right to left) (#168).
  • Prevent the editor from crashing when copy-paste filtering removes all of its content (thibaudcolas/draftjs-filters@652750f)

Out of all of these the ones that aren't complete fringe cases are #4985, and "fix entity removal when selection is backwards".

New features

  • Add support for Markdown shortcuts for inline styles, e.g. ** for bold, _ for italic, etc (#134, #187). View the full list of keyboard shortcuts.

New APIs

  • Add onFocus and onBlur props to use callbacks on those events. This can be useful for form validation. #170, #174, thanks to @TheSpicyMeatball.
  • Add plugins API to support extensions of the editor using the draft-js-plugins architecture (#83, #171).
  • Add ability to disable or customise the editor toolbar with topToolbar.
  • Add ability to add a toolbar below the editor with bottomToolbar.
  • Add data reset parameter to DraftUtils.resetBlockWithType().
  • Enable list continuation on Enter for custom *-list-item blocks. All that’s required is for the block type to end with -list-item.

None of those API additions will be usable within Wagtail (at least without hacks) until corresponding rich text features APIs are built to allow their configuration. The last 2 additions would already be usable but they would only be useful when leveraging the other APIs.


  • Do the tests still pass? (http://docs.wagtail.io/en/latest/contributing/developing.html#testing)
  • [ ] Does the code comply with the style guide? (Run make lint from the Wagtail root)
  • [ ] For Python changes: Have you added tests to cover the new/fixed behaviour?
  • For front-end changes: Did you test on all of Wagtail’s supported browsers? See below. All but mobile browsers
  • [ ] For new features: Has the documentation been updated accordingly?
  • Chrome Desktop 72 macOS 10.14.1
  • Chrome Desktop 71 Windows 10
  • IE Desktop 11
  • MS Edge Desktop 18
  • MS Edge Desktop 17
  • Firefox Desktop 65 macOS 10.14.1
  • Firefox ESR Desktop 60 Windows 10
  • Safari macOS 12.0.1
  • Safari macOS 11.1
  • Safari iOS 12
  • Safari iOS 11
  • Chrome Android 68
@thibaudcolas

This comment has been minimized.

Copy link
Member Author

thibaudcolas commented Mar 6, 2019

While doing further testing I spotted a regression in how keyboard shortcuts are enabled 😑 I’ll publish v1.2.1 and update the PR shortly.

@thibaudcolas thibaudcolas force-pushed the thibaudcolas:feature/draftail-upgrade-4985 branch 3 times, most recently from 90a3caf to 36e3909 Mar 7, 2019

@thibaudcolas

This comment has been minimized.

Copy link
Member Author

thibaudcolas commented Mar 7, 2019

Ok, the regression with bold/italic/code/underline being always available as keyboard shortcuts should be fixed, and I finished testing on mobile devices. @jonnyscholes @xfxf please go ahead if you can have a look at this and test it a bit.

Beyond the usual manual smoke testing, the interesting bits to test would be:

  • #4985 – Prevent crash when filtering pasted content whose last block is to be removed (e.g. unsupported image) (#179).
  • Fix entity removal and editing not doing anything when the selection is backwards (right to left) (#168).
  • Add support for Markdown shortcuts for inline styles, e.g. ** for bold, _ for italic, etc (#134, #187). View the full list of keyboard shortcuts.

@thibaudcolas thibaudcolas force-pushed the thibaudcolas:feature/draftail-upgrade-4985 branch from 36e3909 to c888722 Mar 15, 2019

@thibaudcolas thibaudcolas changed the title Upgrade Draftail to v1.2.0. Fix #4985, adds more Markdown shortcuts Upgrade Draftail to v1.2.1. Fix #4985, adds more Markdown shortcuts Mar 18, 2019

@jonnyscholes

This comment has been minimized.

Copy link
Member

jonnyscholes commented Mar 19, 2019

Tested the above in Chrome on MacOS. All 3 of the referenced issues seem to be resolved/completed. One note about bold/italic shortcuts is they only worked for me if I didnt end my text in a space. eg **asdf** worked but **asdf ** did not. Is that a known limitation @thibaudcolas?

@thibaudcolas

This comment has been minimized.

Copy link
Member Author

thibaudcolas commented Mar 20, 2019

@jonnyscholes 👌

For the Markdown, yes, this is done on purpose. I tried to find a balance between too eagerly matching patterns that people didn't mean to be Markdown, and not matching things that are valid Markdown but could also be something else. The end result is inspired by Dropbox Paper, which has a similar behavior for whitespace.

Here is the magical regex: https://regexper.com/#%28%5Cs%7C%5E%29__%28%5B%5E%5Cs_%5D%7B1%2C2%7D%7C%5B%5E%5Cs_%5D.%2B%5B%5E%5Cs_%5D%29__%24

This is applied to text from the beginning of the block that has focus, to where the cursor is, including the character that's typed when triggering the matching.

If anyone wants to test this matching, https://demo.draftail.org/storybook/?path=/story/draftail--simple is a good place.

@jonnyscholes

This comment has been minimized.

Copy link
Member

jonnyscholes commented Mar 21, 2019

Cool - thanks for the links @thibaudcolas. React storybook is brilliant!

Upgrade Draftail to v1.2.1. Fix #4985
This includes:

- [v1.0.0](https://github.com/springload/draftail/releases/tag/v1.0.0) (identical to v0.17.2)
- [v1.1.0](https://github.com/springload/draftail/releases/tag/v1.1.0) (contains fix for #4985)
- [v1.2.0](https://github.com/springload/draftail/releases/tag/v1.2.0)
- [v1.2.1](https://github.com/springload/draftail/releases/tag/v1.2.1) (fixes regression in v1.2.0)

Here is the combined CHANGELOG for what's relevant to Wagtail:

Bug fixes
~~~~~~~~~

- #4985 – Prevent crash when filtering pasted content whose last block is to be removed (e.g. unsupported image) ([#179](springload/draftail#179)).
- Stop unnecessarily calling `onSave` in the editor’s `onBlur` ([#173](springload/draftail#173)).
- Prevent crash in `DraftUtils.getEntitySelection`, when the provided entity key isn't valid (undefined, missing) ([#168](springload/draftail#168)).
- Fix entity removal and editing not doing anything when the selection is backwards (right to left) ([#168](springload/draftail#168)).
- Prevent the editor from crashing when copy-paste filtering removes all of its content (thibaudcolas/draftjs-filters@652750f)

New features
~~~~~~~~~~~~

- Add support for Markdown shortcuts for inline styles, e.g. `**` for bold, `_` for italic, etc ([#134](springload/draftail#134), [#187](springload/draftail#187)). View the full list of [keyboard shortcuts](https://www.draftail.org/docs/keyboard-shortcuts).

New APIs
~~~~~~~~

- Add [`onFocus`](https://www.draftail.org/docs/api#managing-focus) and [`onBlur`](https://www.draftail.org/docs/api#managing-focus) props to use callbacks on those events. This can be useful for [form validation](https://www.draftail.org/docs/next/form-validation). [#170](springload/draftail#170), [#174](springload/draftail#174), thanks to [@TheSpicyMeatball](https://github.com/TheSpicyMeatball).
- Add [`plugins`](https://www.draftail.org/docs/plugins) API to support extensions of the editor using the [draft-js-plugins](https://github.com/draft-js-plugins/draft-js-plugins) architecture ([#83](springload/draftail#83), [#171](springload/draftail#171)).
- Add ability to disable or customise the editor toolbar with [`topToolbar`](https://www.draftail.org/docs/customising-toolbars).
- Add ability to add a toolbar below the editor with [`bottomToolbar`](https://www.draftail.org/docs/customising-toolbars).
- Add data reset parameter to `DraftUtils.resetBlockWithType()`.
- Enable list continuation on Enter for custom `*-list-item` blocks. All that’s required is for the block type to end with `-list-item`.

None of those API additions will be usable within Wagtail (at least without hacks) until corresponding rich text features APIs are built to allow their configuration. The last 2 additions would already be usable but they would only be useful when leveraging the other APIs.
@thibaudcolas

This comment has been minimized.

Copy link
Member Author

thibaudcolas commented Mar 21, 2019

🙂 love Storybook as well! It's been a real time saver.

I've done one more round of testing and all seems ok, will now proceed to merge this.

@thibaudcolas thibaudcolas force-pushed the thibaudcolas:feature/draftail-upgrade-4985 branch from c888722 to 7d8d191 Mar 21, 2019

@thibaudcolas thibaudcolas merged commit f987fa9 into wagtail:master Mar 21, 2019

2 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
ci/circleci: backend Your tests passed on CircleCI!
Details
ci/circleci: frontend Your tests passed on CircleCI!
Details

@thibaudcolas thibaudcolas deleted the thibaudcolas:feature/draftail-upgrade-4985 branch Mar 21, 2019

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.