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.

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

JS Cleanup #198

Draft
wants to merge 10 commits into
base: development
Choose a base branch
from
3 changes: 3 additions & 0 deletions validation/jinja2/validation/_base.html
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,11 @@
<script>
const csrftoken = document.querySelector('[name=csrfmiddlewaretoken]').value;
</script>
<script src="{{ static('helpers.js') }}"></script>
<script src="{{ static('index.js') }}"></script>
<script src="{{ static('menu.js') }}"></script>
<script src="{{ static('editSegment.js') }}"></script>
<script src="{{ static('toggleWrongWordDiv.js') }}"></script>
<title>{% block title %} Maskwacîs Recordings Validation {% endblock title %}</title>
{% endblock head %}

Expand Down
4 changes: 2 additions & 2 deletions validation/jinja2/validation/_menu.html
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@
<ul id="options-menu" class="menu__options menu__options--list menu__options--right">
{% if auth %}
{% if request.COOKIES['macron'] == 'off' %}
<li onclick="showMacron('on')"><a href="#" class="menu__options-link">Show ê with circumflex</a></li>
<li data-cy="show-flex-button"><a href="#" class="menu__options-link">Show ê with circumflex</a></li>
{% else %}
<li onclick="showMacron('off')"><a href="#" class="menu__options-link">Show e without circumflex</a></li>
<li data-cy="no-flex-button"><a href="#" class="menu__options-link">Show e without circumflex</a></li>
{% endif %}
<li><a href="/logout" id="logout-link" class="logout-link">Logout</a></li>

Expand Down
7 changes: 4 additions & 3 deletions validation/jinja2/validation/_wrong_buttons.html
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,17 @@

{% macro button_col(recording, csrf_token, request) %}

<button class="button {{ 'button--neutral-solid' if recording.wrong_speaker else 'button--neutral' }} wrong-speaker-button" data-cy="wrong-button" data-toggle="modal" data-target="#{{ recording.id }}-modal" data-rec-id={{ recording.id }}>Wrong Speaker</button>
<button class="button {{ 'button--neutral-solid' if recording.wrong_word else 'button--neutral' }} wrong-word-button" data-cy="wrong-button" data-rec-id={{ recording.id }} onclick="showWrongWordDiv('{{ recording.id }}')">Wrong Word</button>
<button class="button {{ 'button--neutral-solid' if recording.wrong_speaker else 'button--neutral' }} wrong-speaker-button" data-cy="wrong-speaker-button" data-toggle="modal" data-target="#{{ recording.id }}-modal" data-rec-id={{ recording.id }}>Wrong Speaker</button>
<button class="button {{ 'button--neutral-solid' if recording.wrong_word else 'button--neutral' }} wrong-word-button" data-cy="wrong-word-button" data-rec-id={{ recording.id }}>Wrong Word</button>

<div class="menu__none rec-wrong-word" data-rec-id={{ recording.id }}>
<form id="{{ recording.id }}-wrong-word" method="POST" action="/api/save_wrong_word/{{ recording.id }}">
<input type="hidden" name="csrfmiddlewaretoken" value="{{ csrf_token }}">
<input type="hidden" name="referrer" value="{{ request.build_absolute_uri() }}">
What word is being spoken?
<input type="text" id="wrong_word" name="wrong_word">
<input form="{{ recording.id }}-wrong-word" type="submit" value="Save" class="button button--success button--small save-wrong-word" data-rec-id={{ recording.id }}>
<input type="button" class="button button--fail button--small" value="Cancel" data-rec-id={{ recording.id }})" onclick="hideWrongWordDiv('{{ recording.id }}')">
<input type="button" class="button button--fail button--small" value="Cancel" data-cy="cancel-wrong-word-button" data-rec-id={{ recording.id }})">
Copy link
Member

Choose a reason for hiding this comment

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

Should this be a <button>?

Suggested change
<input type="button" class="button button--fail button--small" value="Cancel" data-cy="cancel-wrong-word-button" data-rec-id={{ recording.id }})">
<button type="button" class="button button--fail button--small" data-cy="cancel-wrong-word-button" data-rec-id={{ recording.id }})">Cancel</button>

</form>
</div>

Expand Down
77 changes: 31 additions & 46 deletions validation/jinja2/validation/segment_details.html
Original file line number Diff line number Diff line change
@@ -1,42 +1,3 @@
<script>
function showDiv(name) {
document.getElementById('edit').style.display = "block";
document.getElementById('id_cree').focus()

const translation = document.getElementById(name + '-translation').innerHTML;

document.getElementById('id_cree').value = name;
document.getElementById('id_translation').value = translation;
}

function showDivAccept(name, translation, analysis) {
document.getElementById('edit').style.display = "block";
document.getElementById('id_cree').focus()

document.getElementById('id_cree').value = name;
document.getElementById('id_translation').value = translation;
document.getElementById('id_analysis').value = analysis;
}

function hideDiv() {
document.getElementById('edit').style.display = "none";
}

function revert(transcription, translation, analysis) {
document.getElementById('edit').style.display = "block";
document.getElementById('id_cree').focus()

document.getElementById('id_cree').value = transcription;
document.getElementById('id_translation').value = translation;
document.getElementById('id_analysis').value = analysis;
}

function goBack() {
window.history.back()
}


</script>

{% extends 'validation/_base.html' %}
{% import 'validation/_macros.html' as macros %}
Expand All @@ -45,11 +6,13 @@
<div class="table-responsive">

<div>
<button class="button button--success button--back" onclick='goBack()'>Back</button>
<button data-cy="back-button" class="button button--success button--back">Back</button>
Copy link
Member

Choose a reason for hiding this comment

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

This is a really pendatic note, but this should be an <a> with the back page as its href. If you know what page it should go "back" to in advance, that href can be hardcoded. Otherwise, this answer from StackOverflow is really nice:

// adapted from: https://stackoverflow.com/a/46163215/6626414
for (let el of document.querySelectorAll('a.button--back')) {
    el.setAttribute('href', document.referrer);

    // this remains the same!
    el.addEventListener('click', (evt) => {
        window.history.back();
    })
}

{% if auth %}
<button data-cy="edit-button" class="button button--fail button--right button--edit"
onclick="showDiv('{{ segment_name }}')"
class="button button--fail button--right">
<button
data-cy="edit-button"
data-segment-name="{{ segment_name }}"
class="button button--fail button--right button--edit"
>
Edit
</button>
{% endif %}
Expand Down Expand Up @@ -123,7 +86,14 @@
<td>
{% set transl = suggestions[suggestion]["matches"][0]["translations"] %}
{% set analysis = suggestions[suggestion]["matches"][0]["analysis"] %}
<input type="button" class="button button--success" value="Accept" onclick="showDivAccept('{{ suggestion }}', '{{ transl }}', '{{ analysis }}')" />
<input
type="button"
data-cy="accept-button"
class="button button--success"
value="Accept"
data-transcription="{{ suggestion }}"
data-translation="{{ transl }}"
data-analysis="{{ analysis }}" />
</td>
{% endif %}
{% endif %}
Expand All @@ -137,7 +107,14 @@
<td>
{% set transl = match["translations"] %}
{% set analysis = match["analysis"] %}
<input type="button" class="button button--success" value="Accept" onclick="showDivAccept('{{ suggestion }}', '{{ transl }}', '{{ analysis }}')" />
<input
type="button"
class="button button--success"
value="Accept"
data-cy="accept-button"
data-transcription="{{ suggestion }}"
data-translation="{{ transl }}"
data-analysis="{{ analysis }}"/>
</td>
</tr>
{% endif %}
Expand Down Expand Up @@ -168,7 +145,15 @@ <h4>Revision History:</h4>
<td data-cy="revision-analysis">{{ revision.analysis }} </td>
{% if auth %}
<td>
<input type="button" class="button button--fail" value="Revert" onclick="revert('{{ revision.transcription }}', '{{ revision.translation }}', '{{ revision.analysis }}')" />
<input
type="button"
class="button button--fail"
value="Revert"
data-cy="revert-button"
data-transcription="{{ revision.transcription }}"
data-translation="{{ revision.translation }}"
data-analysis="{{ revision.analysis }}"
/>
</td>
{% endif %}

Expand Down
4 changes: 2 additions & 2 deletions validation/jinja2/validation/segment_edit.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ <h5>Edit Segment</h5>
<input type="hidden" name="csrfmiddlewaretoken" value="{{ csrf_token }}">
{{ form }}
<input data-cy="save-button" type="submit" class="button button--success" value="Save">
<input data-cy="cancel-button" type="button" class="button button--fail" onclick="hideDiv()" value="Cancel">
<input data-cy="cancel-button" type="button" class="button button--fail" value="Cancel">
Copy link
Member

Choose a reason for hiding this comment

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

Should this be a <button>?

Suggested change
<input data-cy="cancel-button" type="button" class="button button--fail" value="Cancel">
<button type="button" data-cy="cancel-button" type="button" class="button button--fail">Cancel</button>

</form>
</div>
</div>
46 changes: 46 additions & 0 deletions validation/static/editSegment.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
"use strict";

document.addEventListener('DOMContentLoaded', () => {
for (let button of document.querySelectorAll('[data-segment-name]')) {
button.addEventListener("click", async (e) => {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
button.addEventListener("click", async (e) => {
button.addEventListener("click", (e) => {

showDiv()
const name = e.target.dataset.segmentName
const translation = document.getElementById(name + '-translation').innerHTML;

document.getElementById('id_cree').value = name;
document.getElementById('id_translation').value = translation;
})
}

for (let button of document.querySelectorAll('[data-cy="accept-button"], [data-cy="revert-button"]')) {
button.addEventListener("click", async(e) => {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
button.addEventListener("click", async(e) => {
button.addEventListener("click", (e) => {

showDiv()

const transcription = e.target.dataset.transcription
const translation = e.target.dataset.translation
const analysis = e.target.dataset.analysis
Comment on lines +19 to +21
Copy link
Contributor

@andrewdotn andrewdotn May 18, 2021

Choose a reason for hiding this comment

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

If you want to use some fancier syntax*

Suggested change
const transcription = e.target.dataset.transcription
const translation = e.target.dataset.translation
const analysis = e.target.dataset.analysis
const { transcription, translation, analysis } = e.target.dataset;

*I think this has now has excellent browser support


document.getElementById('id_cree').value = transcription;
document.getElementById('id_translation').value = translation;
document.getElementById('id_analysis').value = analysis;
})
}

for (let button of document.querySelectorAll('[data-cy="cancel-button"]')) {
button.addEventListener("click", async(e) => {
document.getElementById('edit').style.display = "none";
})
}

for (let button of document.querySelectorAll('[data-cy="back-button"]')) {
button.addEventListener("click", async(e) => {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
button.addEventListener("click", async(e) => {
button.addEventListener("click", (e) => {

window.history.back()
})
}
})


function showDiv() {
document.getElementById('edit').style.display = "block";
document.getElementById('id_cree').focus()
Copy link
Member

Choose a reason for hiding this comment

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

Note: assigning focus programmatically is considered not a great practice, but here, I don't think it matters much. Always be careful with focus stealing.

}
22 changes: 22 additions & 0 deletions validation/static/helpers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
"use strict";

// Helper functions that are needed across multiple JS files


function getElementByPhraseId(className, phraseId) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You should be able to do a lookup using the DOM API instead of having to do a linear scan:

document.querySelector(`.${className}[data-phrase-id=${phraseId}]`)

That said, I know there were issues with quoting related to the format of the ID that may make that trickier. That’s one reason that automatic numeric ID columns are so handy.

const elements = document.getElementsByClassName(className);
for (let e of elements) {
if (e.dataset.phraseId === phraseId) {
return e
}
}
}

function getElementByRecordingId(className, recordingId) {
const elements = document.getElementsByClassName(className);
for (let e of elements) {
if (e.dataset.recId === recordingId) {
return e
}
}
}
28 changes: 0 additions & 28 deletions validation/static/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,31 +102,3 @@ document.addEventListener('DOMContentLoaded', () => {
}
}
})

function showWrongWordDiv(recordingId) {
const wrongWordDiv = getElementByRecordingId("rec-wrong-word", recordingId);
wrongWordDiv.classList.replace("menu__none", "menu__block");
}

function hideWrongWordDiv(recordingId) {
const wrongWordDiv = getElementByRecordingId("rec-wrong-word", recordingId);
wrongWordDiv.classList.replace("menu__block", "menu__none");
}

function getElementByPhraseId(className, phraseId) {
const elements = document.getElementsByClassName(className);
for (let e of elements) {
if (e.dataset.phraseId === phraseId) {
return e
}
}
}

function getElementByRecordingId(className, recordingId) {
const elements = document.getElementsByClassName(className);
for (let e of elements) {
if (e.dataset.recId === recordingId) {
return e
}
}
}
24 changes: 14 additions & 10 deletions validation/static/menu.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
"use strict";

function showMenu() {
let menu = document.getElementById("options-menu");
if (menu.classList.contains("menu__none")) {
menu.classList.remove("menu__none");
menu.classList.add("menu__block");
} else {
menu.classList.add("menu__none");
menu.classList.remove("menu__block");
document.addEventListener("DOMContentLoaded", () => {
for (let link of document.querySelectorAll('[data-cy="show-flex-button"]')) {
Copy link
Member

Choose a reason for hiding this comment

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

So "flex" button refefers to 'flex as in "cirum" and not as in "-box". Because I thought this was a like a button { display: flex; } at first 😂 😂 😂

link.addEventListener("click", () => {
showCircumflex("on");
})
}
}

function showMacron(option) {
for (let link of document.querySelectorAll('[data-cy="no-flex-button"]')) {
link.addEventListener("click", () => {
showCircumflex("off");
})
}
})


function showCircumflex(option) {
document.cookie = "macron=" + option + "; SameSite=Lax;"
let menu = document.getElementById("options-menu");
menu.classList.add("menu__none");
Expand Down
28 changes: 28 additions & 0 deletions validation/static/toggleWrongWordDiv.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
"use strict";

document.addEventListener("DOMContentLoaded", () => {
for (let button of document.querySelectorAll('[data-cy="wrong-word-button"]')) {
button.addEventListener("click", async (e) => {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
button.addEventListener("click", async (e) => {
button.addEventListener("click", (e) => {

const recordingId = e.target.dataset.recId;
showWrongWordDiv(recordingId);
})
}

for (let button of document.querySelectorAll('[data-cy="cancel-wrong-word-button"]')) {
button.addEventListener("click", async (e) => {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
button.addEventListener("click", async (e) => {
button.addEventListener("click", (e) => {

const recordingId = e.target.dataset.recId;
hideWrongWordDiv(recordingId);
})
}
});


function showWrongWordDiv(recordingId) {
const wrongWordDiv = getElementByRecordingId("rec-wrong-word", recordingId);
wrongWordDiv.classList.replace("menu__none", "menu__block");
}

function hideWrongWordDiv(recordingId) {
const wrongWordDiv = getElementByRecordingId("rec-wrong-word", recordingId);
wrongWordDiv.classList.replace("menu__block", "menu__none");
}