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.

Sign up for GitHub

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

Jump to bottom

fix: fix formactions template to consider arguments #144

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

fix: fix formactions template to consider arguments #144

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2caacfe
fix: fix formactions template to consider arguments
mschoettle Apr 27, 2023
3e75dbc
tests: add tests for FormActions
mschoettle Aug 12, 2023
7378045
Apply suggestions from code review
mschoettle Aug 13, 2023
b75622a
Merge remote-tracking branch 'origin/main' into fix-formactions
mschoettle Oct 31, 2023
9364c20
Merge remote-tracking branch 'origin/main' into fix-formactions
mschoettle Nov 10, 2023
71b1631
bring back extra field_class div
mschoettle Nov 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 4 additions & 7 deletions crispy_bootstrap5/templates/bootstrap5/layout/formactions.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
<div{% if formactions.attrs %} {{ formactions.flat_attrs }}{% endif %} class="mb-3">
{% if label_class %}
<div class="aab {{ label_class }}"></div>
{% endif %}

<div class="{{ field_class }}">
Copy link
Member

Choose a reason for hiding this comment

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

I think we still need to add field_class.

Copy link
Author

@mschoettle mschoettle Aug 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I applied all the suggestions but then thought after that field_class might not be appropriate here: Would field_class not be applied to each child field of FormActions?

Consider the following test:

def test_formactions_field_class(self):
        test_form = SampleForm()
        test_form.helper = FormHelper()
        test_form.helper.form_tag = False
        test_form.helper.field_class = "field-class"
        test_form.helper.layout = Layout(
            FormActions(
                Field('email'),
            ),
        )

        print(render_crispy_form(test_form))

Output:

<div
     
    class="mb-3  field-class" 
    > <div id="div_id_email" class="mb-3"> <label for="id_email" class="form-label requiredField">
                email<span class="asteriskField">*</span> </label> <div class="field-class"> <input type="text" name="email" maxlength="30" class="textinput textInput inputtext form-control" required id="id_email"> <div id="hint_id_email" class="form-text">Insert your email</div> </div> </div> </div>

As per field.html the <input> gets wrapped into a div with CSS class field-class.

Copy link
Member

Choose a reason for hiding this comment

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

Is FormActions used to wrap Fields? The crispy-form docs say...

FormActions: It wraps fields in a

. It is usually used to wrap form’s buttons.

With an example form as per the docs. (please forgive the lack of style on the cancel button).

Main branch:

image

This patch:

image

This therefore looks like a visual regression to me.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks! I reverted that change and also brought back the div for label_class to be consistent with the other template packs.

<div
{% if formactions.flat_attrs %}{{ formactions.flat_attrs }}{% endif %}
class="mb-3{% if 'form-horizontal' in form_class %} row{% endif %} {{ formactions.css_class|default:'' }} {{ field_class }}"
{% if formactions.id %} id="{{ formactions.id }}"{% endif %}>
{{ fields_output|safe }}
</div>
</div>
5 changes: 5 additions & 0 deletions tests/results/test_formactions.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<div
test="formactions-test"
class="mb-3 formactions-test-class field-class"
id="formactions-test-id"> <b>test</b>
</div>
45 changes: 44 additions & 1 deletion tests/test_layout_objects.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
Alert,
AppendedText,
FieldWithButtons,
FormActions,
InlineCheckboxes,
InlineField,
InlineRadios,
Expand Down Expand Up @@ -199,7 +200,6 @@ def test_custom_django_widget(self):
form.helper.layout = Layout("inline_radios")

html = render_crispy_form(form)
print(html)
assert 'class="form-check-input"' in html

# Make sure an inherited CheckboxSelectMultiple gets rendered as it
Expand Down Expand Up @@ -579,3 +579,46 @@ def test_grouped_checkboxes_radios(self):
else:
expected = "test_grouped_radios_failing.html"
assert parse_form(form) == parse_expected(expected)

def test_formactions(self):
test_form = SampleForm()
test_form.helper = FormHelper()
test_form.helper.form_tag = False
test_form.helper.layout = Layout(
FormActions(
HTML("<b>test</b>"),
),
)

assert 'class="mb-3 "' in render_crispy_form(test_form)

def test_formactions_attrs(self):
test_form = SampleForm()
test_form.helper = FormHelper()
test_form.helper.form_tag = False
mschoettle marked this conversation as resolved.
Show resolved Hide resolved
test_form.helper.field_class = "field-class"
test_form.helper.layout = Layout(
FormActions(
HTML("<b>test</b>"),
css_class="formactions-test-class",
css_id="formactions-test-id",
test="formactions-test",
),
)

assert parse_form(test_form) == parse_expected("test_formactions.html")

def test_formactions_horizontal_form(self):
test_form = SampleForm()
test_form.helper = FormHelper()
test_form.helper.form_tag = False
test_form.helper.form_class = 'form-horizontal'
test_form.helper.layout = Layout(
FormActions(
HTML("<b>test</b>"),
css_class="formactions-test-class",
),
)

expected_class = 'class="mb-3 row formactions-test-class "'
assert expected_class in render_crispy_form(test_form)