Skip to content

Commit

Permalink
Fix to ensure added forms can be deleted
Browse files Browse the repository at this point in the history
For `<template>` elements, the `template.content` property returns a
DocumentFragment instead of a Node. These can be added easily to the
DOM using `appendChild`, but deleting them using `removeChild` is
non-trivial in such cases, as pointed out it #6.

To overcome this problem, the `template.content.firstElementChild` is
added to the DOM instead, which is a Node. This does require the
template element to contain only a single element and the library now
also checks for this upon initialization, but in practice this was
already the case.

End-to-end tests are updated to assert this check is performed and to
ensure added forms can indeed be deleted. The documentation is updated
to clarify that each form should be contained inside a single parent
element denoted by the `formSelector` parameter.
  • Loading branch information
tiesjan committed Feb 7, 2023
1 parent be54381 commit 34319d8
Show file tree
Hide file tree
Showing 5 changed files with 115 additions and 4 deletions.
20 changes: 20 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,26 @@ See the example in the Quick start guide above on how to render the formset in
your HTML template. Feel free to add some intermediate DOM elements if it suits
your template better, as long as you stick to the basic structure shown above.

It is important that both the visible forms and the empty form follow the same
structure. A form also needs to be contained inside a single parent element,
denoted by the _**formSelector**_ parameter:

```htmldjango
<!-- Visible forms inside forms container -->
<div id="email-forms-container">
<div class="email-form">
<!-- Form -->
</div>
</div>
<!-- Empty form inside template element -->
<template id="empty-form-template">
<div class="email-form">
<!-- Form -->
</div>
</template>
```

#### Configuration
Creating an instance of the JavaScript constructor function `ConvenientFormset`
allows a user to add and delete forms within the rendered formset. When a user
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -528,9 +528,12 @@ const ConvenientFormset = function(options) {
* Initializes `formsetElements` by selecting DOM elements from the
* formset using the specified selectors.
*
* Missing DOM elements will throw an error.
* Missing DOM elements and malformed empty form template will throw an
* error.
*/
let emptyFormChildElementCount = -1;
const missingElements = [];

let selector;

// Select DOM elements
Expand All @@ -543,11 +546,16 @@ const ConvenientFormset = function(options) {
if (formsetOptions.canAddForms) {
selector = formsetOptions.emptyFormTemplateSelector;
const emptyFormTemplate = document.querySelector(selector);
if (emptyFormTemplate === null || emptyFormTemplate.content === undefined) {
if (emptyFormTemplate === null) {
missingElements.push(selector);
}
else {
formsetElements.emptyForm = emptyFormTemplate.content;
emptyFormChildElementCount = emptyFormTemplate.content.childElementCount;
if (emptyFormChildElementCount === 1) {
formsetElements.emptyForm = (
emptyFormTemplate.content.firstElementChild
);
}
}

selector = formsetOptions.addFormButtonSelector;
Expand All @@ -568,6 +576,16 @@ const ConvenientFormset = function(options) {
);
throw new Error(message);
}

// Throw error if the empty form template is malformed
else if (formsetOptions.canAddForms && emptyFormChildElementCount !== 1) {
const message = (
'Expected 1 element inside ' +
`"${formsetOptions.emptyFormTemplateSelector}", ` +
`found: ${emptyFormChildElementCount}`
);
throw new Error(message);
}
}

function selectManagementFormElements() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
{% extends "base.html" %}


{% block page_scripts %}
<script>
window.addEventListener('load', function(event) {
new ConvenientFormset({
'formsetPrefix': 'formset',
'formsContainerSelector': '#formset #forms-container',
'formSelector': '.form',

'canAddForms': true,
'addFormButtonSelector': '#formset #add-form-button',
'emptyFormTemplateSelector': '#formset #empty-form-template',
'hideAddFormButtonOnMaxForms': true,

'canDeleteForms': false,

'canOrderForms': false,
});
});
</script>
{% endblock%}


{% block page_contents %}
<div id="formset">
<div id="forms-container"></div>
<input type="button" id="add-form-button" value="Add form">
<template id="empty-form-template">
<div class="form">
<input type="text" name="formset-__prefix__-user" value="">
</div>
<div class="another-form">
<input type="text" name="formset-__prefix__-user" value="">
</div>
</template>
<div id="management-form">
<input type="hidden" name="formset-TOTAL_FORMS" value="0">
<input type="hidden" name="formset-INITIAL_FORMS" value="0">
<input type="hidden" name="formset-MIN_NUM_FORMS" value="0">
<input type="hidden" name="formset-MAX_NUM_FORMS" value="5">
</div>
</div>
{% endblock %}
22 changes: 22 additions & 0 deletions py_tests/end_to_end_tests/test_initialization.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,28 @@ def test_missing_formset_elements3(live_server, selenium):
]


def test_malformed_empty_form_template(live_server, selenium):
"""
Asserts the ConvenientFormset instantiation raises an error message when
the <template> element with `emptyFormTemplateSelector` contains more than
1 child element.
"""
# Load webpage for test
params = {'template_name': 'initialization/malformed_empty_form_template.html'}
test_url = f'{live_server.url}?{urlencode(params)}'
selenium.get(test_url)

# Assert errors
error_log = selenium.find_element(By.CSS_SELECTOR, '#error-log')
error_messages = [
msg.strip() for msg in error_log.text.split('\n') if msg.strip()
]
assert error_messages == [
'[ConvenientFormset] Expected 1 element inside '
'"#formset #empty-form-template", found: 2'
]


def test_missing_management_form(live_server, selenium):
"""
Asserts the ConvenientFormset instantiation raises an error message when
Expand Down
8 changes: 7 additions & 1 deletion py_tests/end_to_end_tests/test_interaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -393,10 +393,16 @@ def test_combined_form_actions(live_server, selenium):
forms[0].find_element(By.CSS_SELECTOR, '#move-form-down-button').click()
forms[4].find_element(By.CSS_SELECTOR, '#move-form-up-button').click()

# Initiate click on add form button
# Initiate two clicks on add form button
add_form_button = selenium.find_element(
By.CSS_SELECTOR, '#formset #add-form-button')
add_form_button.click()
add_form_button.click()

# Delete the last form, to assert that deleting a newly added forms works
forms = selenium.find_elements(
By.CSS_SELECTOR, '#formset #forms-container .form')
forms[-1].find_element(By.CSS_SELECTOR, '#delete-form-button').click()

# Assert errors
error_log = selenium.find_element(By.CSS_SELECTOR, '#error-log')
Expand Down

0 comments on commit 34319d8

Please sign in to comment.