wagtail / wagtail

A Django content management system focused on flexibility and user experience
https://wagtail.org
BSD 3-Clause "New" or "Revised" License
18.39k stars 3.9k forks source link

Stop importing _editor_js.html everywhere #2936

Open gasman opened 8 years ago

gasman commented 8 years ago

wagtailadmin/pages/_editor_js.html was only ever meant to be used on the page add/edit views, but is currently imported in various templates throughout Wagtail admin because it's a convenient grab-bag of JS functions that saves the effort of actually thinking about what JS you're using. Some of the code there is generally reusable (e.g. page chooser modals) and some of it is relevant to other edit-handler-based interfaces (snippets, settings) but a lot of it is only specific to the page editor (e.g. setting up behaviour for the slug field and preview button). We really ought to refactor these templates so that they only import and run the JS that's actually meaningful to each page.

gasman commented 7 years ago

Have been looking into what it would take to allow AdminPageChooser to be used on pages outside the page editor, without importing all of _editor_js.html, and it seems like it's going to be more involved than just shuffling some import lines around...

AdminPageChooser requires the following JS code to work:

We could easily split these off into a new _page_chooser_js.html include, included from _editor_js.html and any other templates that require AdminPageChooser. However, this is going to lead us down a path of messy dependency tracking: a page that includes _page_chooser_js.html has to know not to import modal-workflow.js itself. And if we do the same decoupling exercise on another component, such as the image chooser, there's no way to include both of them without importing modal-workflow.js twice.

To make this work, then, we'll need a helper that keeps track of the JS imports that are already present, and de-duplicates them - something like Django's form media, but globally for the template, and able to handle arbitrary snippets such as the one for window.chooserUrls, not just <script> file imports. Perhaps the building block would be a template tag like:

{% includeonce chooser_urls %}
    <script>
        window.chooserUrls = {...};
    </script>
{% endincludeonce %}
{% includeonce modal_workflow %}
    <script src="{% static 'modal-workflow.js' %}"></script>
{% endincludeonce %}
thibaudcolas commented 7 years ago

This sounds like the textbook use case for a module system, with a bundler like Webpack. I think we should move the dependency graph management there, further away from Django land.

I'm not sure how this fits with dynamic snippets that generate arbitrary JS.

lb- commented 2 years ago

Flagging as potentially in scope of the RFC78 stimulus project - much of the JS in this file will be replaced with Stimulus controller logic most likely.

lb- commented 1 year ago

@Lovelyfin00 please have a read of this issue and also a read of what's in this file. I'm hopeful we can get a solid chunk of this done before the Stimulus Outreachy project finishes.

We have discussed the individual JS items before and they are on the roadmap spreadsheet. However, I think it would be good for you to take a fresh look at the things inside this file and how they could be migrated to Stimulus.

lb- commented 1 year ago

Have created a few issues (Stimulus) that will help us move towards removing page-editor.js and then removing _editor_js.html usage.

One thing to consider is the hooks (editor js/css) and expectations of behaviour for that.

lb- commented 9 months ago

Update

lb- commented 9 months ago

With https://github.com/wagtail/wagtail/pull/11620 no Wagtail admin core code will use the hook "insert_editor_js" and we will also clean up even more usage of the _editor_js.html template.

Which is an awesome win, in doing this we get closer to supporting CSP and have provided a better way for RichText to leverage custom URL values and be used in more cases across the admin.

lb- commented 3 weeks ago

Another PR up to get us closer to this https://github.com/wagtail/wagtail/pull/12496

lb- commented 2 weeks ago

I am thinking, maybe the comments.js could use the Form media instead of being declared in editor_js.

https://github.com/wagtail/wagtail/blob/246f3c7eb542be2081556bf5334bc22256776bf4/wagtail/admin/templates/wagtailadmin/pages/_editor_js.html#L7