Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
3 changes: 2 additions & 1 deletion app/eventyay/cfp/templates/cfp/event/submission_base.html
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
{% include "cfp/includes/forms_header.html" %}
{% compress js %}
<script defer src="{% static "cfp/js/proposalTabTitles.js" %}"></script>
<script defer src="{% static "cfp/js/formAutoSave.js" %}"></script>
Copy link
Member

Choose a reason for hiding this comment

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

Please use different quote style when nesting.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, thanks

{% endcompress %}
{% block cfp_submission_header %}{% endblock cfp_submission_header %}
{% endblock cfp_header %}
Expand All @@ -39,7 +40,7 @@
</div>
{% block cfp_form %}
{% if request.user.is_authenticated %}
<form method="post"{% if form.is_multipart %} enctype="multipart/form-data"{% endif %}>
<form id="cfp-submission-form" method="post"{% if form.is_multipart %} enctype="multipart/form-data"{% endif %}>
{% csrf_token %}
{{ wizard.management_form }}
<div class="d-flex align-items-start">
Expand Down
207 changes: 207 additions & 0 deletions app/eventyay/static/cfp/js/formAutoSave.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,207 @@
/**
* Auto-save CfP form data to sessionStorage to preserve it during browser navigation
* This ensures data is not lost when users use the browser back button
*/

(function() {
Copy link
Member

Choose a reason for hiding this comment

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

This style of JS code (wrap the whole code in an anonymous function) is outdated and verbose.
Please use "<script type='module'>".

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably I'm outdated with respect to js ;-)

'use strict';

// Get unique storage key based on the current URL (includes tmpid and step)
function getStorageKey() {
const path = window.location.pathname;
return `cfp_form_data_${path}`;
}

// Debounce function to avoid saving too frequently
function debounce(func, wait) {
let timeout;
return function executedFunction(...args) {
const later = () => {
clearTimeout(timeout);
func(...args);
};
clearTimeout(timeout);
timeout = setTimeout(later, wait);
};
}

// Save form data to sessionStorage
function saveFormData() {
const form = document.getElementById('cfp-submission-form');
if (!form) return;

const formData = {};

// Find all form fields using querySelectorAll for reliability
const textareas = form.querySelectorAll('textarea');
const inputs = form.querySelectorAll('input:not([type="hidden"]):not([type="submit"])');
const selects = form.querySelectorAll('select');

// Process all textareas
textareas.forEach((textarea) => {
const name = textarea.name;
if (name && name !== 'csrfmiddlewaretoken') {
formData[name] = textarea.value;
}
});
Comment on lines +37 to +46
Copy link
Contributor

Choose a reason for hiding this comment

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

issue: Checkbox groups with the same name are not handled correctly in auto-save

The logic currently saves a single boolean per checkbox name, so groups with shared names overwrite each other and are all restored to the same value. For checkbox groups, you should instead persist an array of selected values (like multi-selects) and, on restore, set each checkbox’s checked state based on whether its value is in that array.


// Process all inputs
inputs.forEach((input) => {
const name = input.name;
const type = input.type;

if (!name || name === 'csrfmiddlewaretoken' || name === 'action') return;

if (type === 'checkbox') {
Comment on lines +48 to +55
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (bug_risk): Checkbox groups with the same name are not handled correctly when saving/restoring.

The current logic stores a single boolean per checkbox name (formData[name] = input.checked), so in a group only the last processed checkbox controls the stored value. On restore, that single boolean is then applied to every checkbox in the group, losing the actual selection.

For checkbox groups that represent multi-select fields, store an array of checked values keyed by name, similar to multiple selects, and restore based on value:

// saveFormData
if (type === 'checkbox') {
    if (!formData[name]) formData[name] = [];
    if (input.checked) formData[name].push(input.value);
}
// restore
if (elements.length > 1 && elements[0].type === 'checkbox' && Array.isArray(value)) {
    for (let i = 0; i < elements.length; i++) {
        elements[i].checked = value.includes(elements[i].value);
    }
    continue;
}

This preserves the correct set of checked options per group.

formData[name] = input.checked;
} else if (type === 'radio') {
if (input.checked) {
formData[name] = input.value;
}
} else if (type !== 'file') {
formData[name] = input.value;
}
});

// Process all selects
selects.forEach((select) => {
const name = select.name;
if (name && name !== 'csrfmiddlewaretoken') {
if (select.multiple) {
formData[name] = Array.from(select.selectedOptions).map(opt => opt.value);
} else {
formData[name] = select.value;
}
}
});

try {
sessionStorage.setItem(getStorageKey(), JSON.stringify(formData));
} catch (e) {
console.warn('Failed to save form data to sessionStorage:', e);
}
}

// Restore form data from sessionStorage
function restoreFormData() {
try {
const savedData = sessionStorage.getItem(getStorageKey());
if (!savedData) return;

const formData = JSON.parse(savedData);
const form = document.getElementById('cfp-submission-form');
if (!form) return;

// Check if the saved sessionStorage data matches the current form data
// If they match exactly, it means we successfully submitted and the server
// echoed back our data - in this case, clear sessionStorage
let allFieldsMatch = true;
let checkedFields = 0;

for (const [name, savedValue] of Object.entries(formData)) {
const elements = form.elements[name];
if (!elements) continue;

const element = elements.length !== undefined ? elements[0] : elements;
const currentValue = element.value || '';
const savedValueStr = String(savedValue || '');
Comment on lines +98 to +107
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (bug_risk): The "all fields match" detection for clearing sessionStorage is likely too weak and may never trigger.

This comparison only uses element.value from the first control per name, so it ignores checked state for checkboxes/radios and multi-select values, and may treat matching submissions as mismatches. That means allFieldsMatch can stay false even when the server echoed the submitted data, leaving stale sessionStorage around.

To make this reliable, derive currentData the same way as in saveFormData() and compare the serialized forms (e.g. JSON.stringify(currentData) === savedData) instead of relying on form.elements[name][0].value.

Suggested implementation:

            // Check if the saved sessionStorage data matches the current form data.
            // If they match exactly, it means we successfully submitted and the server
            // echoed back our data - in this case, clear sessionStorage.
            // We reconstruct the current form data in the same shape as what we store.
            const currentData = {};
            const formDataFromDom = new FormData(form);

            for (const [name, value] of formDataFromDom.entries()) {
                if (Object.prototype.hasOwnProperty.call(currentData, name)) {
                    const existing = currentData[name];
                    if (Array.isArray(existing)) {
                        existing.push(value);
                    } else {
                        currentData[name] = [existing, value];
                    }
                } else {
                    currentData[name] = value;
                }
            }

            const serializedCurrentData = JSON.stringify(currentData);

            // If the serialized current form data matches what we have stored,
            // it means the form was successfully submitted - clear sessionStorage.
            if (serializedCurrentData === savedData) {
                clearFormData();
                return;
            }

To be fully robust and future-proof, you should ensure that saveFormData() uses the same normalization/serialization logic as this block. The ideal approach is:

  1. Extract the currentData building logic into a shared helper (e.g. getNormalizedFormData(form)).
  2. Use that helper both in saveFormData() (before sessionStorage.setItem) and here (before comparison), so any changes to how form data is represented are made in one place.


checkedFields++;
if (currentValue.trim() !== savedValueStr.trim()) {
Comment on lines +101 to +110
Copy link

Copilot AI Dec 20, 2025

Choose a reason for hiding this comment

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

The comparison 'currentValue.trim() !== savedValueStr.trim()' assumes both values are strings with a trim() method. However, if the currentValue is from a checkbox or other non-text input, it might not have a trim() method. This could cause a runtime error. Consider adding type checks or handling non-string values appropriately.

Copilot uses AI. Check for mistakes.
allFieldsMatch = false;
break;
}
}

// If all saved fields match current values AND we checked some fields,
// it means the form was successfully submitted - clear sessionStorage
if (allFieldsMatch && checkedFields > 0) {
clearFormData();
return;
}
Comment on lines +95 to +121
Copy link

Copilot AI Dec 20, 2025

Choose a reason for hiding this comment

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

The logic here checks if allFieldsMatch and clears sessionStorage if true. However, this assumes that a match between saved and current values means successful submission. This could lead to false positives in edge cases where the user manually enters the exact same values that were in sessionStorage, then navigates back before submitting. Consider adding an additional indicator (e.g., a flag in sessionStorage) to explicitly track successful form submission.

Copilot uses AI. Check for mistakes.

// Otherwise, restore from sessionStorage
for (const [name, value] of Object.entries(formData)) {
const elements = form.elements[name];
if (!elements) continue;

// Handle NodeList (radio buttons, checkboxes with same name)
if (elements.length > 1) {
for (let i = 0; i < elements.length; i++) {
const element = elements[i];
if (element.type === 'checkbox') {
element.checked = value;
} else if (element.type === 'radio') {
element.checked = (element.value === value);
}
}
} else {
const element = elements.length !== undefined ? elements[0] : elements;
Comment on lines +101 to +139
Copy link

Copilot AI Dec 20, 2025

Choose a reason for hiding this comment

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

The logic for checking if an element is a NodeList and accessing the first element is duplicated in lines 105 and 139. The pattern 'elements.length !== undefined ? elements[0] : elements' appears twice. Consider extracting this into a helper function to reduce duplication and improve maintainability.

Copilot uses AI. Check for mistakes.
const type = element.type;

if (type === 'checkbox') {
element.checked = value;
} else if (type === 'radio') {
element.checked = (element.value === value);
} else if (element.tagName === 'SELECT') {
if (element.multiple && Array.isArray(value)) {
for (let i = 0; i < element.options.length; i++) {
element.options[i].selected = value.includes(element.options[i].value);
}
} else {
element.value = value;
}
} else if (element.tagName === 'TEXTAREA' || element.tagName === 'INPUT') {
// Always restore - sessionStorage takes precedence
// If we got this far, the matching logic already determined
// this is new data that should be restored
element.value = value;
}
}
Comment on lines +124 to +160
Copy link

Copilot AI Dec 20, 2025

Choose a reason for hiding this comment

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

The code iterates through form.elements[name] which can be either a single element or a NodeList depending on whether multiple elements share the same name. The check 'elements.length !== undefined' is used to distinguish these cases, but this is fragile. NodeList and HTMLCollection both have a length property, but a single element does not. However, checking for 'undefined' specifically could fail if length exists but is 0. Consider using a more robust check like 'elements instanceof NodeList || elements instanceof RadioNodeList' or 'elements.tagName === undefined' to distinguish collections from single elements.

Copilot uses AI. Check for mistakes.
}
} catch (e) {
console.warn('Failed to restore form data from sessionStorage:', e);
}
}

// Clear saved form data (called after successful submission)
function clearFormData() {
try {
sessionStorage.removeItem(getStorageKey());
} catch (e) {
console.warn('Failed to clear form data from sessionStorage:', e);
}
}

// Initialize auto-save functionality
function init() {
const form = document.getElementById('cfp-submission-form');
if (!form) return;

// Restore form data on page load
restoreFormData();

// Create debounced save function (save 500ms after user stops typing)
const debouncedSave = debounce(saveFormData, 500);

// Attach event listeners to form elements
form.addEventListener('input', debouncedSave);
form.addEventListener('change', debouncedSave);

// Save before page unload (browser back button, refresh, close tab, etc.)
// This ensures data is preserved even when using browser navigation
window.addEventListener('beforeunload', saveFormData);

// Note: We don't clear sessionStorage on form submit because:
// 1. We can't reliably detect if submission succeeded (might have validation errors)
// 2. The restore logic already handles this by clearing sessionStorage when
// it detects the form has server data (successful previous submission)
}

// Initialize when DOM is ready
if (document.readyState === 'loading') {
document.addEventListener('DOMContentLoaded', init);
} else {
init();
}
})();
11 changes: 8 additions & 3 deletions app/eventyay/static/cfp/js/proposalTabTitles.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@ const updateTitle = (newTitle) => {
}

const checkForTitle = () => {
const titleInput = document.getElementById("id_title").value
updateTitle(titleInput)
const titleInput = document.getElementById("id_title")
if (titleInput) {
updateTitle(titleInput.value)
}
}

if (titleParts.length !== 3) {
Expand All @@ -19,5 +21,8 @@ if (titleParts.length !== 3) {
)
} else {
onReady(checkForTitle)
document.getElementById("id_title").addEventListener("change", (ev) => { updateTitle(ev.target.value) })
const titleInput = document.getElementById("id_title")
if (titleInput) {
titleInput.addEventListener("change", (ev) => { updateTitle(ev.target.value) })
}
}