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

Remove the Hallo Editor - now provided as an external package #7922

Merged
merged 6 commits into from Mar 2, 2022

Conversation

lb-
Copy link
Member

@lb- lb- commented Feb 6, 2022

How to test this

  • Check out this branch locally
  • Important: Delete the built static assets at wagtail/admin/static and then run the Wagtail build pipeline nvm use then npm run build - this is required so that validation can be done without the styles/JS provided by Wagtail and only the ones provided by the new package.
  • Then install the package as per the documentation (to also test these instructions make sense), remember to update your bakerydemo (or other local Wagtail instance) settings to use the hallo editor
  • You may need to make a new page that uses the hallo editor, here are two example models.

@squash-labs
Copy link

squash-labs bot commented Feb 6, 2022

Manage this branch in Squash

Test this branch here: https://lb-featurehallo-editor-removal-fp9zn.squash.io

@thibaudcolas
Copy link
Member

@lb- looks like it’s all passing :) I won’t be able to review this this week but should be able to help from next week onwards.

@lb- lb- force-pushed the feature/hallo-editor-removal branch from 2502d31 to e224f79 Compare February 7, 2022 22:14
@thibaudcolas thibaudcolas self-requested a review February 9, 2022 10:04
@lb- lb- force-pushed the feature/hallo-editor-removal branch 2 times, most recently from fe8b7f8 to c268b47 Compare February 12, 2022 13:03
@lb- lb- force-pushed the feature/hallo-editor-removal branch 2 times, most recently from 208cb4b to a48e704 Compare February 15, 2022 09:53
@lb- lb- marked this pull request as ready for review February 15, 2022 09:57
@lb-
Copy link
Member Author

lb- commented Feb 15, 2022

@thibaudcolas ready for a review now
cc @zerolab

@lb- lb- changed the title WIP - Feature/hallo editor removal Remove the Hallo Editor - now provided as an external package Feb 16, 2022
@lb- lb- force-pushed the feature/hallo-editor-removal branch from a48e704 to f9093b3 Compare February 16, 2022 09:39
@lb-
Copy link
Member Author

lb- commented Feb 16, 2022

rebased on main & fixed up the conflicts with the black formatter change

@lb- lb- force-pushed the feature/hallo-editor-removal branch from f9093b3 to fd64207 Compare February 16, 2022 09:50
docs/extending/rich_text_internals.rst Outdated Show resolved Hide resolved
docs/releases/2.17.md Outdated Show resolved Hide resolved
docs/releases/2.17.md Outdated Show resolved Hide resolved
docs/releases/2.17.md Outdated Show resolved Hide resolved
@lb- lb- force-pushed the feature/hallo-editor-removal branch from 6a890e7 to 295289a Compare February 16, 2022 11:54
@lb-
Copy link
Member Author

lb- commented Mar 1, 2022

Reminder to self - rebase needed

Copy link
Member

@thibaudcolas thibaudcolas left a comment

Choose a reason for hiding this comment

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

Looking excellent @lb-. I’ve picked up very minor things. I think they’re worth fixing but should be very straightforward and this will be good to merge afterwards. This is as thorough of a cleanup as I’ve ever seen!

lb-was-here

I have yet to try the wagtail-hallo package, will get to it next, considering the feedback on #6228 I think we can merge this already and if anything further comes up we can pick it up in wagtail-hallo.

docs/extending/index.rst Show resolved Hide resolved
docs/spelling_wordlist.txt Outdated Show resolved Hide resolved
wagtail/core/rich_text/feature_registry.py Outdated Show resolved Hide resolved
wagtail/core/rich_text/feature_registry.py Show resolved Hide resolved
wagtail/core/tests/test_rich_text.py Outdated Show resolved Hide resolved
wagtail/documents/rich_text/editor_html.py Outdated Show resolved Hide resolved
wagtail/embeds/rich_text/editor_html.py Outdated Show resolved Hide resolved
wagtail/images/rich_text/editor_html.py Outdated Show resolved Hide resolved
@lb- lb- force-pushed the feature/hallo-editor-removal branch from 4bbf31c to 51a519e Compare March 1, 2022 21:49
@lb- lb- requested a review from thibaudcolas March 1, 2022 21:49
@lb-
Copy link
Member Author

lb- commented Mar 1, 2022

@thibaudcolas thanks, I did my best to get the draftail.js to be consistently Draftail as part of this - some comments have been moved to pydoc style module descriptions instead as that seemed more appropriate.

Can you take another look please, I am happy to merge in but just wanted to get another pass.

Thanks heaps

Copy link
Member

@thibaudcolas thibaudcolas left a comment

Choose a reason for hiding this comment

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

giphy-dancingpineapple

@lb- lb- force-pushed the feature/hallo-editor-removal branch from 51a519e to 7d425f4 Compare March 2, 2022 10:15
@lb- lb- merged commit aaddf81 into wagtail:main Mar 2, 2022
@thibaudcolas thibaudcolas added this to the 2.17 milestone Mar 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove support for Hallo editor
5 participants