django-crispy-forms / crispy-tailwind

A Tailwind template pack for django-crispy-forms
MIT License
331 stars 56 forks source link

Django 4.0 Support (formerly fix CheckboxSelectMultiple widget) #116

Closed grundleborg closed 2 years ago

grundleborg commented 2 years ago

Because this is a subclass of RadioSelect, the check for is_radioselect needs to come after the check for is_checkboxselectmultiple otherwise the radio widget will always be shown instead.

Fixes #115

codecov-commenter commented 2 years ago

Codecov Report

Merging #116 (809c16c) into main (370ccb9) will increase coverage by 0.02%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #116      +/-   ##
==========================================
+ Coverage   82.41%   82.44%   +0.02%     
==========================================
  Files          37       37              
  Lines         637      638       +1     
  Branches       28       34       +6     
==========================================
+ Hits          525      526       +1     
  Misses        102      102              
  Partials       10       10              
Impacted Files Coverage Δ
crispy_tailwind/templates/tailwind/field.html 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 370ccb9...809c16c. Read the comment docs.

smithdc1 commented 2 years ago

Thanks for this. 👍

Yes, this changed in Django 4.0, see https://github.com/django/django/commit/b9e872b59329393f615c440c54f632a49ab05b78

Could you add a test for this? I'd like to have a test to ensure we continue to support Django 3.2.

grundleborg commented 2 years ago

Yes happy to add a test - I'll update this PR.

I'm finding, though, that the current test suite already has some failures with Django 4.x - I'll see if I can send a separate PR for that first.

grundleborg commented 2 years ago

Actually, on closer inspection it turns out there was already a test for this, which was failing on Django 4.0. I've changed the CI to include Django 4.0 and fixed the other tests that were broken and included it all in this PR.

grundleborg commented 2 years ago

@smithdc1 would you mind approving the updates to this PR for the CI to run when you have a moment? I think this should fix all the django 4.0 issues now (including the original one I reported with CheckboxSelectMultiple).

grundleborg commented 2 years ago

Ah sorry, I messed up in the CI config - this time it really should work!

smithdc1 commented 2 years ago

Thanks for the updates. Could you also update the setup.py file to update the classifiers for the newly supported versions?

Thanks again.

grundleborg commented 2 years ago

Ah yeah, I forgot about that - now added to the PR.

smithdc1 commented 2 years ago

Thanks for pushing this along. I've pushed a few edits to bump versions of other items across the package. Let's see what the tests says.

smithdc1 commented 2 years ago

@grundleborg thank you for all of your efforts. 🏅

grundleborg commented 2 years ago

You're welcome - it's a very useful library so thank you for creating it.