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

Wagtail edit HTML bug #1415

Closed
Tracked by #113
patphongs opened this issue Oct 30, 2017 · 6 comments
Closed
Tracked by #113

Wagtail edit HTML bug #1415

patphongs opened this issue Oct 30, 2017 · 6 comments
Assignees
Labels
Milestone

Comments

@patphongs
Copy link
Member

After the release, the edit HTML button no longer works. There appears to be an error in the hallo-edit-html.js when trying to use that button.

screen shot 2017-10-30 at 11 14 19 am

@ccostino
Copy link
Contributor

ccostino commented Nov 6, 2017

@johnnyporkchops and I tried to work through this issue this morning but didn't have much success. We talked about why the proposed hotfix isn't ideal as it is copying only parts of the module directly into build pipeline-managed directories (separate note in the PR about that) and that it is likely a config issue of sort.

My intuition is telling me it's something we ought to be able to configure via webpack to ensure the module is built and found appropriately, but I'm not 100% sure and I am also not sure how to get webpack to recognize the module at the moment; I'd need to spend some time reading the documentation to help fix this.

@xtine
Copy link
Contributor

xtine commented Nov 6, 2017

The actual issue here is that pre-merge, fec-cms would include all node_modules in collectstatic, so we could reference ace-builds directly. However after merge we decided to remove that line so we have to figure out how to include only ace-builds in collectstatic.

@ccostino
Copy link
Contributor

ccostino commented Nov 6, 2017

Thank you again, @xtine, for spending time helping investigate the root cause of this! It's also possible that ace-builds isn't the only module to have suffered this fate, but it's the only one we've seen explicit behavior missing for now. We should at least check the other modules within the wagtail_hooks.py file to ensure those are all still working as intended as a part of determining how to fix this.

@xtine
Copy link
Contributor

xtine commented Nov 6, 2017

@ccostino: I believe this is the only module affected. In wagtail_hooks.py, all the other JS includes start with /static/js/* and exist in /fec/fec/static/js/*.

Related to this, it would probably be helpful to have some documentation somewhere that explains these custom additions to Wagtail admin. It looks like we have hallo and rangy modules that could also be hooked in as NPM modules than relying on dropped in vendor files, and also the one-off jquery.htmlclean and beautify-html files. What do these all do? @johnnyporkchops would you be interested in investigating these?

@johnnyporkchops
Copy link
Contributor

Per @Xtines comments above: Yes, I will familiarize myself with the files called by wagtail_hooks.py so I can report to you guys,what does what, when as we determine the best solution to the html-edit/ace-builds issue.

@johnnyporkchops
Copy link
Contributor

johnnyporkchops commented Nov 8, 2017

@xtine mentioned in standup that a temporary solution miiiiiiight be necessary in the short term. There is one temp solution that would include changing only one file and not building in all of the ace-builds library. That would be replacing our custom /fec/static/js/admin/hallo-edit-html.js with the stock hallo-edit-html.js. This would allow html editing, but would have no syntax highlighting that ace provides. I am thinking that because it is only on file, it is unobtrusive enought to revert without worrying about managing a bunch of library fiiles that get pulled into the build.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants