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

Widget no longer renders for blocks in Wagtail 6.1 #11935

Closed
enzedonline opened this issue May 7, 2024 · 3 comments
Closed

Widget no longer renders for blocks in Wagtail 6.1 #11935

enzedonline opened this issue May 7, 2024 · 3 comments
Assignees
Labels

Comments

@enzedonline
Copy link

enzedonline commented May 7, 2024

There seems to have been a change to widget loading in blocks not covered in the release notes. I have a block with custom widget that has loaded fine until Wagtail 6.1. Since upgrade, only the widget associated with the inherited Wagtail block type is rendered.

I've seen the note for initBlockWidget, but this isn't apparently relevant to this case.

I have a block that inherits TextBlock and overrides the widget (code working in 6.0.x):

Steps to Reproduce

class ImportTextBlock(TextBlock):
    """
    TextArea block with option to import from file or drag/drop.
    file_type_filter: any valid accept string
    https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/accept
    """
    def __init__(
        self,
        required=True,
        help_text=None,
        rows=5,
        max_length=None,
        min_length=None,
        file_type_filter=None,
        validators=(),
        **kwargs
    ):
        super().__init__(
            required, help_text, rows, max_length, min_length, validators, **kwargs
        )
        self.file_type_filter = file_type_filter

    @cached_property
    def field(self):
        field_kwargs = {
            "widget": ImportTextAreaWidget(
                self.file_type_filter, 
                attrs={"rows": self.rows}
            )
        }
        field_kwargs.update(self.field_options)
        return forms.CharField(**field_kwargs)

    class Meta:
        icon = "copy"

and widget

class ImportTextAreaWidget(AdminAutoHeightTextInput):
    def __init__(self, file_type_filter=None, attrs={}):
        self.accept = file_type_filter
        if not attrs.get("rows"):
            attrs["rows"] = 5
        super().__init__(attrs)

    msg = {
        "help": _("Use 'Choose File' or drag/drop to import data from file."),
        "button_label": _("Choose file"),
    }

    def render(self, name, value, attrs, renderer=None):
        context = {
            "id": f'{attrs.get("id")}',
            "label": self.msg["button_label"],
            "help": self.msg["help"],
            "accept": self.accept,
        }
        return super().render(name, value, attrs, renderer) + render_to_string(
            "widgets/import_textarea_widget.html", context
        )

    @cached_property
    def media(self):
        widget_media = super().media
        return forms.Media(
            js=widget_media._js + ["js/widgets/import-textarea-widget.js"],
            css={"all": ("css/widgets/import-textarea-widget.css",)},
        )

The render() method adds a file picker button and some other html then initialises a javascript class to enable the file reading capabilities.

<div class="import-textarea-fileinput-container">
    <input type="file"
           hidden=""
           id="{{ id }}-file-input"
           {% if accept %}accept="{{ accept }}"{% endif %}
    >
    <label class="button" for="{{ id }}-file-input">{{ label }}</label>
    <span class="help">{{ help }}</span>
    <script>new ImportTextAreaWidget('{{ id }}')</script>
</div>

This has been in use since Wagtail 3.x.

Since upgrade from 6.0 to 6.1, the additional html is stripped out on the rendered form. The widget render method returns:

<textarea name="__NAME__" cols="40" rows="5" data-controller="w-autosize" class="w-field__autosize" id="__ID__">
</textarea>
<div class="import-textarea-fileinput-container">
    <input type="file"
           hidden=""
           id="__ID__-file-input"
           accept=".csv"
    >
    <label class="button" for="__ID__-file-input">Choose file</label>
    <span class="help">Use &#x27;Choose File&#x27; or drag/drop to import data from file.</span>
    <script>new ImportTextAreaWidget('__ID__')</script>
</div>

On the form, only the textarea is rendered (confirmed by manipulating the returned html - this comes from the custom widget):

 <textarea name="__NAME__" cols="40" rows="5" data-controller="w-autosize" class="w-field__autosize" id="__ID__">
</textarea>

The additional content in the <div> container has been stripped.

The issue is not javascript related since none of the additional html in the render method is present in the rendered form (and no console errors).

Putting breakpoints in the ImportTextBlock field method, and the widget render method, I can see the code is being called as expected during page load.

This issue only exists for blocks, the widget renders as expected for FieldPanels associated with a TextField.

  • I have confirmed that this issue can be reproduced as described on a fresh Wagtail project: (yes )

Technical details

  • Python version: 3.12
  • Django version: 5.0.3
  • Wagtail version: 6.1.0
@enzedonline enzedonline added status:Unconfirmed Issue, usually a bug, that has not yet been validated as a confirmed problem. type:Bug labels May 7, 2024
@laymonage laymonage removed the status:Unconfirmed Issue, usually a bug, that has not yet been validated as a confirmed problem. label May 16, 2024
@laymonage
Copy link
Member

laymonage commented May 16, 2024

Thanks for the report! I can confirm this issue. It's a regression in 9b74c83, particularly the change I pointed out in https://github.com/wagtail/wagtail/pull/11839/files#r1566809981.

In short: your customisation here concatenates the rendered original TextInput with your custom template. As a result, the rendered HTML has multiple top-level nodes. The JS code was changed in the above commit to only take the first node, hence the issue.

As a workaround, you can change the way you render the template by moving the original textarea into your <div> wrapper. For example, update your widgets/import_textarea_widget.html to:

<div class="import-textarea-fileinput-container">
	{% comment %}
        "wagtailadmin/shared/attrs.html" is identical to "django/forms/widgets/attrs.html",
         which is what the original Textarea widget uses
    {% endcomment %}
    <textarea name="{{ widget.name }}"{% include "wagtailadmin/shared/attrs.html" %}>
        {% if widget.value %}{{ widget.value }}{% endif %}</textarea>
    <input type="file"
           hidden=""
           id="{{ id }}-file-input"
           {% if accept %}accept="{{ accept }}"{% endif %}
    >
    <label class="button" for="{{ id }}-file-input">{{ label }}</label>
    <span class="help">{{ help }}</span>
    <script>new ImportTextAreaWidget('{{ id }}')</script>
</div>

Then update your widget code to:

class ImportTextAreaWidget(AdminAutoHeightTextInput):
    template_name = "widgets/import_textarea_widget.html"

    def __init__(self, file_type_filter=None, attrs={}):
        self.accept = file_type_filter
        if not attrs.get("rows"):
            attrs["rows"] = 5
        super().__init__(attrs)

    msg = {
        "help": ("Use 'Choose File' or drag/drop to import data from file."),
        "button_label": ("Choose file"),
    }

    def get_context(self, name, value, attrs):
        context = super().get_context(name, value, attrs)
        context.update(
            {
                "id": f'{attrs.get("id")}',
                "label": self.msg["button_label"],
                "help": self.msg["help"],
                "accept": self.accept,
            }
        )
        return context

    def render(self, name, value, attrs, renderer=None):
        return render_to_string(
            self.template_name,
            self.get_context(name, value, attrs),
        )

Unrelated note: after setting template_name and moving the context to get_context(), the render() override can be removed entirely if your widget template is located inside app_name/templates/widgets/import_textarea_widget.html. If it's located somewhere else, using render_to_string is necessary to use your template settings, as by default the FORM_RENDERER setting uses django.forms.renderers.DjangoTemplates which may be different to your settings. It's also why I used wagtailadmin/shared/attrs.html instead of django/forms/widgets/attrs.html, which wouldn't work if render_to_string() isn't used.

Since we've discussed before @gasman, do you think we should reinstate support for multiple top-level nodes as a bugfix? Or shall we label this as an enhancement?

@enzedonline
Copy link
Author

Great, thanks for the update and info.

I'll hold off upgrading until the fix/change is included in the current release. I've got some other 3rd party Django widgets in use affected by this, I'd need a wrapper for each otherwise.

Thanks also for the code suggestions above - much appreciated.

@laymonage laymonage self-assigned this May 20, 2024
@gasman
Copy link
Collaborator

gasman commented May 21, 2024

Fixed in #11958

@gasman gasman closed this as completed May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

3 participants