From 26eac87c057e3e38463c428d58261b999d484668 Mon Sep 17 00:00:00 2001
From: Niklas Mohrin <dev@niklasmohrin.de>
Date: Mon, 25 Oct 2021 21:41:20 +0200
Subject: [PATCH] Fix various small mistakes with keyboard tabbing logic - also
port to TypeScript
- Port keyboard logic for student_vote.html to TypeScript
- Disallow tabbing onto the "additional textanswer" button
- Select the correct input when first tabbing into the form area
- Reload student vote site on form errors
- Before this change, we would use the HTML that we got back from the AJAX submission and just rewrite the DOM with `document.write`. This is ... controversial.
- okay, I guess we can also let people back out of the form again if you really insist
- _inlines return_; niklas: 'is this test driven development?'
- wow, this was broken on master all along
- just throwing requestAnimationFrame at the problem, 100% success rate so far
- Inline variable, outline my skills
- Rename selectable{, s}
---
evap/static/ts/src/student-vote.ts | 157 +++++++++++++++++
evap/student/templates/student_vote.html | 165 ++----------------
.../student_vote_questionnaire_group.html | 2 +-
3 files changed, 173 insertions(+), 151 deletions(-)
create mode 100644 evap/static/ts/src/student-vote.ts
diff --git a/evap/static/ts/src/student-vote.ts b/evap/static/ts/src/student-vote.ts
new file mode 100644
index 000000000..1a047cf41
--- /dev/null
+++ b/evap/static/ts/src/student-vote.ts
@@ -0,0 +1,157 @@
+function isInvisible(el: Element): boolean {
+ if (getComputedStyle(el).display === "none")
+ return true;
+ return el.parentElement !== null && isInvisible(el.parentElement);
+}
+
+function selectByNumberKey(row: HTMLElement, num: number) {
+ let index = 2 * num - 1;
+ if (num === 0) {
+ // Select "No answer"
+ index = row.children.length - 1;
+ }
+
+ if (!(0 <= index && index < row.children.length)) {
+ return;
+ }
+
+ const nextElement = row.children[index] as HTMLElement;
+ nextElement.click();
+}
+
+const studentForm = document.getElementById("student-vote-form") as HTMLElement;
+const selectables: NodeListOf<HTMLElement> = studentForm.querySelectorAll(".tab-selectable");
+const rows = Array.from(studentForm.getElementsByClassName("tab-row")) as Array<HTMLElement>;
+const letterRegex = new RegExp("^[A-Za-zÄÖÜäöü\.\*\+\-]$");
+
+// Sometimes we just want the browser to do it's thing.
+let disableFocusHandler = false;
+
+selectables[0].addEventListener("focus", () => {
+ if (disableFocusHandler) {
+ disableFocusHandler = false;
+ return;
+ }
+
+ requestAnimationFrame(() => {
+ // When first entering the form area with the altered tabbing rules, we
+ // need to make sure that we start on the correct input.
+ const correctInput = findCorrectInputInRow(rows[0]);
+ if (selectables[0] !== correctInput) {
+ fancyFocus(correctInput);
+ }
+ });
+});
+
+studentForm.addEventListener("keydown", (e: KeyboardEvent) => {
+ const current = document.activeElement as HTMLElement;
+ if (!current.matches("input, label, span, textarea, button")) {
+ return;
+ }
+
+ if (current.tagName !== "TEXTAREA") {
+ // We want to disable backspace, because users may think that
+ // they can undo their selection, but they would be sent to the
+ // student index (or where they came from otherwise).
+ // Additionally, pressing Enter shouldn't submit the form.
+ switch (e.key) {
+ case "Enter":
+ current.click(); // fallthrough
+ case "Backspace":
+ e.preventDefault();
+ return;
+ }
+ }
+
+ // Since the event could be caught on either the outer label or
+ // the nested text / input, the full row could be two steps up
+ const currentRow: HTMLElement | null = current.closest(".tab-row");
+ if (currentRow === null) {
+ return;
+ }
+
+ const insideSubmitRow = currentRow.closest(".card-submit-area") !== null;
+ if (!insideSubmitRow && current.tagName !== "TEXTAREA") {
+ const num = parseInt(e.key);
+ if (!isNaN(num)) {
+ // Even if the number does not trigger a selection (for example pressing "9"),
+ // nothing else should happen, because it would be very frustrating if only some numbers
+ // would work as expected.
+ e.preventDefault();
+ selectByNumberKey(currentRow, num);
+ return;
+ }
+ }
+
+ if (e.key !== "Tab") {
+ if (current.tagName !== "TEXTAREA" && letterRegex.test(e.key)) {
+ const wholeRow = currentRow.closest("div.row");
+ if (wholeRow === null)
+ return;
+
+ e.preventDefault();
+ const textAnswerButton: HTMLElement | null = wholeRow.querySelector("[data-bs-toggle=\"collapse\"]");
+ const textField: HTMLTextAreaElement | null = wholeRow.querySelector("textarea.tab-selectable");
+ if (textAnswerButton !== null && textField !== null) {
+ if (isInvisible(textField))
+ textAnswerButton.click();
+ fancyFocus(textField);
+ textField.value += e.key;
+ textField.dispatchEvent(new Event("input"));
+ }
+ }
+ return;
+ }
+
+ const curRowIndex = rows.indexOf(currentRow);
+ const direction = e.shiftKey ? -1 : 1;
+ let nextRowIndex = curRowIndex;
+ do {
+ nextRowIndex += direction;
+
+ if (nextRowIndex === -1) {
+ // User wants to tab out in front of the form area
+ // To correctly select the first element in front of the form,
+ // select the first element tracked by this.
+ // Just giving back control to the browser here doesn't work, because
+ // it would navigate backwards through the controls of the current row.
+ disableFocusHandler = true;
+ selectables[0].focus({ preventScroll: true });
+ return;
+ } else if (nextRowIndex === rows.length) {
+ // User wants to tab out behind the form area
+ selectables[selectables.length - 1].focus({ preventScroll: true });
+ return;
+ }
+ } while (isInvisible(rows[nextRowIndex]));
+
+ e.preventDefault();
+ fancyFocus(findCorrectInputInRow(rows[nextRowIndex]));
+});
+
+function findCorrectInputInRow(row: HTMLElement) {
+ const alreadySelectedElement: HTMLElement = row.querySelector(".tab-selectable:checked")!;
+
+ if (alreadySelectedElement) {
+ return alreadySelectedElement;
+ } else {
+ const possibleTargets: NodeListOf<HTMLElement> = row.querySelectorAll(".tab-selectable");
+ if (possibleTargets.length === 3) {
+ // Yes-No / No-Yes question, should focus first element
+ return possibleTargets[0];
+ } else {
+ // Everything else: The middle of all the answers excluding "no answer"
+ // This also handles all the single possibility cases
+ const index = Math.floor((possibleTargets.length - 1) / 2);
+ return possibleTargets[index];
+ }
+ }
+}
+
+function fancyFocus(element: HTMLElement) {
+ element.focus({ preventScroll: true });
+ element.scrollIntoView({
+ behavior: "smooth",
+ block: "center",
+ });
+}
diff --git a/evap/student/templates/student_vote.html b/evap/student/templates/student_vote.html
index 23dd5ed50..f6bbef4b2 100644
--- a/evap/student/templates/student_vote.html
+++ b/evap/student/templates/student_vote.html
@@ -89,14 +89,14 @@
<div class="card collapsible{% if not forloop.last %} mb-3{% endif %}">
<div class="card-header d-flex tab-row">
<div class="me-auto">
- <button class="collapse-toggle{% if errors_exist and not contributor_has_errors %} collapsed{% endif %} bg-transparent" data-bs-toggle="collapse"
+ <button class="collapse-toggle{% if errors_exist and not contributor_has_errors %} collapsed tab-selectable{% endif %} bg-transparent" data-bs-toggle="collapse"
data-bs-target="#vote-area-{{ contributor.id }}" aria-expanded="false" aria-controls="vote-area-{{ contributor.id }}" tabindex="-1" type="button">
{{ contributor.full_name }}
{% if label %} – <span class="fst-italic">{{ label }}</span>{% endif %}
</button>
</div>
<div>
- <button type="button" class="btn btn-light btn-sm tab-selectable"
+ <button type="button" class="btn btn-light btn-sm{% if not preview %} tab-selectable{% endif %}"
data-mark-no-answers-for="{{ contributor.id }}"
{% if preview %}disabled{% endif %}
>
@@ -275,16 +275,9 @@
sisyphus.manuallyReleaseData();
window.location.replace("{{ success_redirect_url }}");
} else {
- window.scrollTo({
- top: 0,
- behavior: "auto",
- });
- // Manually perform the page load: first replace content
- document.open();
- document.write(data);
- document.close();
- // then set URL (e.g. logout redirect)
- window.history.pushState('', '', xhr.responseURL);
+ // resubmit without this handler to show the site with the form errors
+ form.unbind("submit");
+ form.submit();
}
},
error: function(data) {
@@ -317,7 +310,7 @@
sisyphus.saveAllData();
// hide questionnaire for contributor
- var voteAreaCollapse = bootstrap.Collapse.getOrCreateInstance(voteArea);
+ const voteAreaCollapse = bootstrap.Collapse.getOrCreateInstance(voteArea);
voteAreaCollapse.hide();
collapseToggle.classList.add("tab-selectable");
@@ -334,11 +327,19 @@
button.disabled = false;
});
});
+
+ collapseToggle.addEventListener("click", () => {
+ if (button.classList.contains("tab-selectable")) {
+ collapseToggle.classList.remove("tab-selectable");
+ }
+ });
});
// remove error highlighting when an answer was selected
document.querySelectorAll(".vote-btn.choice-error").forEach(voteButton => {
voteButton.addEventListener("click", () => clearChoiceError(voteButton));
+ const actualInput = document.getElementById(voteButton.attributes["for"].value);
+ actualInput.addEventListener("click", () => clearChoiceError(voteButton));
});
document.querySelectorAll(".btn-textanswer").forEach(textanswerButton => {
@@ -376,143 +377,7 @@
sisyphus.saveAllData();
});
}
-
- function isInvisible(el) {
- if (getComputedStyle(el).display === "none")
- return true;
- return el.parentElement !== null && isInvisible(el.parentElement);
- }
-
- function selectByNumberKey(row, num) {
- let index = 2 * num - 1;
- if (num === 0) {
- // Select "No answer"
- index = row.children.length - 1;
- }
-
- if (!(0 <= index && index < row.children.length)) {
- return;
- }
-
- const nextElement = row.children[index];
- nextElement.click();
- }
-
- const studentForm = document.getElementById("student-vote-form");
- const selectable = studentForm.querySelectorAll(".tab-selectable");
- const rows = Array.from(studentForm.getElementsByClassName("tab-row"));
- const letterRegex = new RegExp("^[A-Za-zÄÖÜäöü\.\*\+\-]$");
- studentForm.addEventListener("keydown", e => {
- const current = document.activeElement;
- if (!current.matches("input, label, span, textarea, button")) {
- return;
- }
-
- if (current.tagName !== "TEXTAREA") {
- // We want to disable backspace, because users may think that
- // they can undo their selection, but they would be sent to the
- // student index (or where they came from otherwise).
- // Additionally, pressing Enter shouldn't submit the form.
- switch (e.key) {
- case "Enter":
- current.click(); // fallthrough
- case "Backspace":
- e.preventDefault();
- return;
- }
- }
-
- // Since the event could be caught on either the outer label or
- // the nested text / input, the full row could be two steps up
- const currentRow = current.closest(".tab-row");
- if (currentRow === null) {
- return;
- }
-
- const insideSubmitRow = currentRow.closest(".card-submit-area") !== null;
- if (!insideSubmitRow && current.tagName !== "TEXTAREA") {
- const num = parseInt(e.key);
- if (!isNaN(num)) {
- // Even if the number does not trigger a selection (for example pressing "9"),
- // nothing else should happen, because it would be very frustrating if only some numbers
- // would work as expected.
- e.preventDefault();
- selectByNumberKey(currentRow, num);
- return;
- }
- }
-
- if (e.key !== "Tab") {
- if (current.tagName !== "TEXTAREA" && letterRegex.test(e.key)) {
- const wholeRow = currentRow.closest("div.row");
- if (wholeRow === null)
- return;
-
- e.preventDefault();
- const textAnswerButton = wholeRow.querySelector("[data-bs-toggle=\"collapse\"]");
- const textField = wholeRow.querySelector("textarea.tab-selectable");
- if (textAnswerButton !== null && textField !== null) {
- if (isInvisible(textField))
- textAnswerButton.click();
- textField.focus({ preventScroll: true });
- textField.scrollIntoView({
- behavior: "smooth",
- block: "center",
- });
- textField.value += e.key;
- textField.dispatchEvent(new Event("input"));
- }
- }
- return;
- }
-
- const curRowIndex = rows.indexOf(currentRow);
- const direction = e.shiftKey ? -1 : 1;
- let nextRowIndex = curRowIndex;
- do {
- nextRowIndex += direction;
-
- if (nextRowIndex === -1) {
- // User wants to tab out in front of the form area
- // To correctly select the first element in front of the form,
- // select the first element tracked by this.
- // Just giving back control to the browser here doesn't work, because
- // it would navigate backwards through the controls of the current row.
- selectable[0].focus({ preventScroll: true });
- return;
- } else if (nextRowIndex === rows.length) {
- // User wants to tab out behind the form area
- selectable[selectable.length - 1].focus({ preventScroll: true });
- return;
- }
- } while (isInvisible(rows[nextRowIndex]));
- const nextRow = rows[nextRowIndex];
-
- e.preventDefault();
- let nextElement;
- const alreadySelectedElement = nextRow.querySelector(".tab-selectable:checked");
-
- if (alreadySelectedElement) {
- nextElement = alreadySelectedElement;
- } else {
- const possibleTargets = nextRow.querySelectorAll(".tab-selectable");
- if (possibleTargets.length === 3) {
- // Yes-No / No-Yes question, should focus first element
- nextElement = possibleTargets[0];
- } else {
- // Everything else: The middle of all the answers excluding "no answer"
- // This also handles all the single possibility cases
- const index = Math.floor((possibleTargets.length - 1) / 2);
- nextElement = possibleTargets[index];
- }
- }
-
- nextElement.focus({ preventScroll: true });
- nextElement.scrollIntoView({
- behavior: "smooth",
- block: "center",
- });
- });
</script>
+ <script type="module" src="{% static 'js/student-vote.js' %}"></script>
{% endif %}
{% endblock %}
diff --git a/evap/student/templates/student_vote_questionnaire_group.html b/evap/student/templates/student_vote_questionnaire_group.html
index a5c27c522..d0726384d 100644
--- a/evap/student/templates/student_vote_questionnaire_group.html
+++ b/evap/student/templates/student_vote_questionnaire_group.html
@@ -25,7 +25,7 @@
</span>
{% if allows_textanswer %}
<span class="my-auto" data-bs-toggle="tooltip" data-container=".col-question" title="{% trans 'Add a text answer to this question' %}">
- <button type="button" class="btn btn-textanswer collapsed" data-bs-toggle="collapse" data-bs-target=".collapse-{{ field.name }}">
+ <button type="button" class="btn btn-textanswer collapsed" data-bs-toggle="collapse" data-bs-target=".collapse-{{ field.name }}" tabindex="-1">
<span class="far fa-comment"></span>
</button>
</span>
--
GitLab