I'm unsure how this fell through the cracks, but WYSIWYG template should not be committed to our repo.
Tasks:
Remove the code
Remove the WYSIWYG template config file from /config/install
Fix our composer.json to pull WYSIWYG template library from the right place.
Open a PR to add the repo here too https://github.com/sparksi/sector-project
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | fix-wysiwyg-template-path_3157340_11.patch | 1.49 KB | danielveza |
| #2 | sector-3157340-composer_ckeditor_templates.patch | 67.95 KB | xurizaemon |
Comments
Comment #2
xurizaemonLooks like the backstory to committing the templates plugin was recorded in #2945979: Allow use with composer without requiring ckeditor/templates.
Taking a stab at a patch for this, but feeling my way as I'm a little unclear about process when the libraries are referenced in composer.json for Sector, Sector Profile AND Sector Distribution :)
Related PRs:
* https://github.com/sparksi/sector-project/pull/5
* https://github.com/sparksi/sector-distribution/pull/10
Comment #3
danielvezaFantastic! Thanks for jumping on this. This is a funny one since it does span Sector project and Sector distro.
The drupal.org and sector-distribution repos are essentially mirrors, so only things needed are the PRs on Sector project and this patch. :).
I'll review this today!
Comment #4
xurizaemonAwesome! If you're able to document / share your process in reviewing this, that'll help me test & review changes to the "recipe" components in future.
Comment #5
dieuweThis is good, although I got a few notices that some of the files in the templates directory had changed. Daniel just needs to check that the "libraries" directory is fully removed and nothing is left when he commits it.
This will require release notes and a notice that the repository will need to be added to all project based on Sector.
As for reviewing changes like this, composer changes are always a bit difficult. I tend to run a fresh checkout, apply the patch, run "composer install", and then make sure it succeeds with the right resulting structure. Then I manually copy the relevant result into an already built site to test the changes.
Comment #6
dieuweI have accepted xurizaemon's pull request to the sector-project default template on Github in preparation for Daniel to commit this patch here.
As it's going to require a change notice and release notes, this will probably be the final patch committed prior to the 5.0 stable release.
Comment #8
dieuweI re-rolled the patch due to composer.json changes and committed it.
Change notice to be published later today.
Comment #9
danielvezaUncaught Error: [CKEDITOR.resourceManager.load] Resource name "templates" was not found at "/profiles/contrib/sector-distribution/libraries/ckeditor/plugins/templates/plugin.js?t=qkfl1o".
This has broken node editing on HEAD. Lets fix that
Comment #10
danielvezaconfig/install/wysiwyg_template.settings.yml
Library path is set to the old path.
We need the following:
Comment #11
danielvezaThis should do the trick
Comment #12
dieuwePatch applies, update runs, and I can put a template in a node. Perfect.
I would recommend removing that empty line at the start of
sector_install()prior to committing the patch, rather than just removing the whitespace.Comment #14
dieuweBack to fixed we go (and hopefully stay there)