Skip to content

Update field_with_buttons.html#44

Merged
smithdc1 merged 4 commits into
django-crispy-forms:mainfrom
digitalfox:patch-1
May 20, 2021
Merged

Update field_with_buttons.html#44
smithdc1 merged 4 commits into
django-crispy-forms:mainfrom
digitalfox:patch-1

Conversation

@digitalfox

Copy link
Copy Markdown
Contributor

Add missing form-label class

Add missing form-label class
@smithdc1

smithdc1 commented Apr 4, 2021

Copy link
Copy Markdown
Member

Thanks. Could you add a test? See #36 for how were currently building tests.

@digitalfox

Copy link
Copy Markdown
Contributor Author

@smithdc1 is it ok for you ?

@smithdc1

smithdc1 commented May 1, 2021

Copy link
Copy Markdown
Member

Thanks for your continued efforts here. 👍

Could we amend the test to test the whole form? It should be "just" a case of putting the html that is in your test into a HTML file and then your assert line can be something like this.

assert parse_form(form) == parse_expected("row.html")

Most of the tests here come from bootstrap4 which were written with partial coverage in code which has been an issue for a long time. We're slowly trying to move to this new method which I'm hoping is more maintainable.

@smithdc1

smithdc1 commented May 5, 2021

Copy link
Copy Markdown
Member

Hi @digitalfox -- I finally got time to sit down with this properly. I think this layout object is one I've not looked at in detail yet. I've pushed a few amends to make it work with bootstap5 button-addons (see link below).

There is also a need to fix the input size for this object but that's a patch for crispy-forms core.

Let me know what you think?

https://getbootstrap.com/docs/5.0/forms/input-group/#button-addons

@smithdc1 smithdc1 merged commit a62505a into django-crispy-forms:main May 20, 2021
@digitalfox

Copy link
Copy Markdown
Contributor Author

Thanks for the merge. Sorry, I did not have time to have a look. I hope to be able to test it next month.

@digitalfox digitalfox deleted the patch-1 branch May 21, 2021 12:18
@smithdc1 smithdc1 added this to the Next Release milestone May 27, 2021
@smithdc1 smithdc1 mentioned this pull request May 30, 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