search

django-crispy-forms / crispy-tailwind

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

[fix](field) change crispy_field to tailwind_field #96

Closed areski closed 2 years ago

areski commented 3 years ago

Fix issue https://github.com/django-crispy-forms/crispy-tailwind/issues/95

smithdc1 commented 3 years ago

Thanks for this. At an initial glance this looks ok. 👍

Could we add some tests to show those templates no longer cause a crash?

codecov-commenter commented 3 years ago

Codecov Report

Merging #96 (3eb488b) into main (2ff9135) will increase coverage by 5.73%. The diff coverage is 90.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #96      +/-   ##
==========================================
+ Coverage   71.11%   76.85%   +5.73%     
==========================================
  Files          37       37              
  Lines         637      635       -2     
  Branches       28       28              
==========================================
+ Hits          453      488      +35     
+ Misses        174      138      -36     
+ Partials       10        9       -1
Impacted Files Coverage Δ
crispy_tailwind/templatetags/tailwind_field.py 50.45% <0.00%> (+1.83%) :arrow_up:
crispy_tailwind/templatetags/tailwind_filters.py 17.30% <ø> (ø)
.../templates/tailwind/layout/field_with_buttons.html 100.00% <100.00%> (+100.00%) :arrow_up:
...ilwind/templates/tailwind/layout/inline_field.html 94.73% <100.00%> (+94.73%) :arrow_up:

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 2ff9135...3eb488b. Read the comment docs.

smithdc1 commented 3 years ago

Hi @areski --thanks for this. I've added some tests and docs for these two fields. For the field with buttons the template also needed a bit of work to make it work for tailwind.

Do you want to take a look at this?

areski commented 3 years ago

Hi @smithdc1 Thanks for jumping on this, sorry I had a busy week and didn't find the time to follow up. really like the tests and the extra documentation. It looks good to me!

areski commented 2 years ago

@smithdc1 I hate to be that guy that put pressure when you have done so much already. Is there anything I can do to help? There is also some other valid PRs that might need some love.

grundleborg commented 2 years ago

Can this PR be merged? I've run into the same issues here and this seems to be a sensible fix.

smithdc1 commented 2 years ago

Thanks for the PR and thanks for the review @grundleborg