Skip to content

Fixed offset class of checkbox in form horizontal#51

Merged
smithdc1 merged 5 commits into
django-crispy-forms:mainfrom
lauri-openscop:patch-1
Jun 3, 2021
Merged

Fixed offset class of checkbox in form horizontal#51
smithdc1 merged 5 commits into
django-crispy-forms:mainfrom
lauri-openscop:patch-1

Conversation

@lauri-openscop

Copy link
Copy Markdown
Contributor

A tricky way to apply the right offset class on checkbox field whitout touching the core application..

A tricky way to apply the right offset class on checkbox field whitout touching the core application..
@lauri-openscop lauri-openscop changed the title Fixed compatibility with bootstrap5 template Fixed offset class of checkbox in form horizontal May 22, 2021
@smithdc1

Copy link
Copy Markdown
Member

Thanks! Glad you found a way of including it in this template pack. 🏅

Could I ask you to add tests for this? See the link below for how we are writing new tests.

#44

Update test_label_class_and_field_class_bs5_offset_when_horizontal to match with the current bootstrap 5 offset
@lauri-openscop

lauri-openscop commented May 25, 2021

Copy link
Copy Markdown
Contributor Author

I have updated test_label_class_and_field_class_bs5_offset_when_horizontal to match with the current offset method. Is that good for you ?

@smithdc1 smithdc1 closed this May 31, 2021
@smithdc1 smithdc1 reopened this May 31, 2021
@smithdc1

smithdc1 commented Jun 1, 2021

Copy link
Copy Markdown
Member

Thanks for this. I think this is close but it doesn't work where the number of columns is two digits. i.e. col-lg-12 would become offset-lg-1.

I was wondering if we could refactor bootstrap_checkbox_offsets in core to be a list/dict(?) of size and number of columns. We could then build the string in the template rather than code as the particular format here seems to change with each major bootstrap release.

What do you think?

@lauri-openscop

lauri-openscop commented Jun 1, 2021

Copy link
Copy Markdown
Contributor Author

My bad, it was cause by the slice. I didn't think about two digit columns. It should work now..
I think that if we have to change the core application the best would be to change helper.py like i said in #49 (comment) or something like that. What do you think ?

@smithdc1

smithdc1 commented Jun 3, 2021

Copy link
Copy Markdown
Member

I think that github asking folk to approve each test run is a bit tiresome (I mean, once would be ok for this PR, but I've pressed it countless times for this PR alone as we've been itterating through the problem!).

As for this patch itself I think it's fine. Let's have this. 👍

@smithdc1 smithdc1 merged commit 80a89e4 into django-crispy-forms:main Jun 3, 2021
@smithdc1 smithdc1 changed the title Fixed offset class of checkbox in form horizontal Fixed offset class of checkbox in form horizontal Jun 3, 2021
@smithdc1 smithdc1 added this to the Next Release milestone Jun 3, 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.

2 participants