django-crispy-forms / crispy-tailwind

A Tailwind template pack for django-crispy-forms
MIT License
356 stars 57 forks source link

Added customised Button classes #28

Closed smithdc1 closed 4 years ago

smithdc1 commented 4 years ago

Currently the project is relying on the layout objects within crispy_forms.

Whilst this works it doesn't make much sense... "why am I importing bootstrap layout objects?".

Further, for buttons default classes are hard coded (uni-form or bootstrap), so we need to override these to add some default Tailwind styles.

For the buttons it adds a default style unless it's been set manually.

codecov-commenter commented 4 years ago

Codecov Report

Merging #28 into master will increase coverage by 3.12%. The diff coverage is 81.81%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #28      +/-   ##
==========================================
+ Coverage   38.59%   41.72%   +3.12%     
==========================================
  Files          35       36       +1     
  Lines         570      592      +22     
  Branches       25       28       +3     
==========================================
+ Hits          220      247      +27     
+ Misses        341      334       -7     
- Partials        9       11       +2     
Impacted Files Coverage Δ
crispy_tailwind/layout.py 81.81% <81.81%> (ø)
..._tailwind/templates/tailwind/layout/baseinput.html 100.00% <0.00%> (+100.00%) :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 05256e9...cccc98b. Read the comment docs.

carltongibson commented 4 years ago

Hi @smithdc1.

I think the core library should provide basic layout objects and reusable Base bits etc.

Then add-ons like this should define their own concrete options and overrides, like you’re suggesting here.

Ideally we’d be able to apply the same patterns to extracting the bootstrap layout objects from the core package...

smithdc1 commented 4 years ago

@carltongibson thank you.

I agree about extracting Bootstrap from the main package. To do this I think we will need to have a look at where some of the layout objects currently are.

Template packs may well want to use InlineRadios / InlineCheckbox (they work just fine 'out of the box'). These can form part of core with the specific layout objects remaining in their own files... but backwards compatibility 😬

I'll rework this and can explain which layout classes to use (and from where) in the docs.

carltongibson commented 4 years ago

I suggest a major version change, with a bit of planning 😀...

If it’s just (cough) a question of changing imports then I think the benefits long-term will justify the breaking changes.

smithdc1 commented 4 years ago

Need to re-read the docs with a fresh pair of eyes.

Hopefully this is more like it 🤞