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

Add support for Wagtail 3 and Wagtail 4 #63

Merged
merged 6 commits into from
Nov 10, 2022
Merged

Conversation

chosak
Copy link
Member

@chosak chosak commented Oct 31, 2022

This PR adds support for Wagtail 3.x and 4.x to this project. It modifies the tox testing matrix to now test against Wagtail 2.15 (LTS), 3.0, and 4.0. It also adds a new tox -e interactive to make it easier to test changes to this package, and bumps test source code coverage to 100%.

Testing

See new test matrix in GitHub Actions.

Notes

Thank you @nickmoreton and @katdom13 for your proposals in #58 and #60; I chose to combine the version upgrades into a single PR along with the other changes described above. If you have a chance to test this branch any feedback would be appreciated.

Todos

Wagtail 4.0 changed the way fields are rendered in wagtail/wagtail@eac5e0b, specifically which broke the (admittedly hacky) hardcoded CSS that breaks how the includes/excludes button was being rendered next to the dropdown:

Wagtail 2.16 Wagtail 3.0 Wagtail 4.0
image image image

Given the interest in adding this version support, and since the form is still useable, I'd prefer not to (further) delay this upgrade because of that issue. I'm hopeful that it shouldn't be that difficult to re-add the previous design, but that would require better understanding of how fields are rendered in 4.x+.

@nickmoreton
Copy link

Hi @chosak thank you doing this. It all seems to be working OK for me.

I had a quick look at the layout of the block inventory page. It's not a perfect fix but at least gets the select dropdown side by side. You can a table around the fields like so.

If you remove the listing class it's even closer in style to v2.16

<div class="wagtail-inventory-select">
      <table class="listing">
        <tr>
          <td>
          {% include "wagtailadmin/shared/field.html" with field=form.has show_label=False field_classes="select-has" %}
          </td>
          <td>
          {% include "wagtailadmin/shared/field.html" with field=form.block show_label=False field_classes="select-block" %}
          </td>
        </tr>
      </table>
    </div>

@chosak
Copy link
Member Author

chosak commented Nov 10, 2022

Thank you @nickmoreton. I spent some time trying to implement a solution that worked well on all supported versions. With 4.0 there's a much simpler way to implement this but unfortunately it's not backwards compatible. In the interest of finally getting this out I'm going to merge this as-is.

@chosak chosak merged commit 30ee26f into main Nov 10, 2022
@chosak chosak deleted the feature/wagtail-3-and-4 branch November 10, 2022 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants