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

Fixed input group size in Bootstrap 4 #1114

Merged
merged 4 commits into from Apr 4, 2021

Conversation

smithdc1
Copy link
Member

@cpina this is still WIP, but thought it worth sharing more publicly my progress with this. Notes:

  • I think your solution for the button groups by adding an extra argument of input_size is spot on. I think we need this for the append inputs as well
  • This is due to bs3 and 4 having different behavior. On bs3 we want the spans and input to have a class, for bs4 we want the wrapping div to have the class (and only the wrapping div).
  • Adding this additional argument allows us to manage this flexibility -- I think we'll also need this for the bs5 pack, so will make things easier there.
  • Field_with_buttons I've not really looked at. Tempted to chop it from this PR and get the rest of it merged.

@codecov-io
Copy link

codecov-io commented Feb 13, 2021

Codecov Report

Merging #1114 (5a3801a) into master (a3946ca) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1114   +/-   ##
=======================================
  Coverage   97.24%   97.25%           
=======================================
  Files          23       23           
  Lines        2836     2844    +8     
  Branches      243      243           
=======================================
+ Hits         2758     2766    +8     
  Misses         38       38           
  Partials       40       40           
Flag Coverage Δ
unittests 97.22% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
crispy_forms/bootstrap.py 90.27% <100.00%> (ø)
crispy_forms/tests/test_layout_objects.py 98.04% <100.00%> (+0.05%) ⬆️

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 a3946ca...5a3801a. Read the comment docs.

@smithdc1 smithdc1 force-pushed the 1080-ds branch 3 times, most recently from 2dd8847 to 6972948 Compare February 14, 2021 14:24
@smithdc1 smithdc1 changed the title [WIP] Fixed input group size in Bootstrap 4 Fixed input group size in Bootstrap 4 Feb 17, 2021
@smithdc1
Copy link
Member Author

OK -- I've trimmed this down to just the append and related classes. We can come back to "with buttons" as a separate change.

@cpina
Copy link
Member

cpina commented Feb 22, 2021

I'll review it, latest on the next weekend and probably and earlier :-)

@smithdc1
Copy link
Member Author

No rush! 🙂

docs/layouts.rst Outdated Show resolved Hide resolved
PrependedText('field_name', StrictButton("Go!"), css_class="input-sm")
PrependedText('field_name', StrictButton("Go!"), css_class="input-lg")

# Bootstrap 4 - Wrapping div needs size class. Use `input_size`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't pass input_size what happens? I thought that it all still works with the default Bootstrap size, right?

Perhaps this could be added here stating that input_size is optional, but that if it's passed then it's used in certain way?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cpina -- you are right but I'm struggling to find the right way to say this succinctly. Can you have a look at my next itteration here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it - thanks!

Copy link
Member

@cpina cpina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've left two small comments about documentation - all the code looks good to me!

@smithdc1 smithdc1 merged commit a7a3b95 into django-crispy-forms:master Apr 4, 2021
@smithdc1 smithdc1 deleted the 1080-ds branch May 5, 2021 08:27
@smithdc1 smithdc1 added this to the Next Release milestone May 25, 2021
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.

Fix class input-group for Prepended_appended_text.html (Bootstrap4)
4 participants