#47 Some issues with voting mechanism

Geschlossen
vor 7 Jahren geöffnet von jorgesumle · 2 Kommentare

There is some inline JavaScript in the voting mechanism: onclick="vote ('down', this.parentNode)", which prevents the voting system to work with LibreJS and also doesn't separate the HTML from the JS. That code would be better in freepost.js, assigning the event onclick for each element using a for loop. Using something like this (I haven't tested it, it just an idea):

var voteUpLinks = document.getElementsByClassName('vote-up');
for (let voteUpLink in voteUpLinks) {
    voteUpLink.onclick = function(){ vote (up, this.parentNode)};
}

var voteDownLinks = document.getElementsByClassName('vote-down');
for (let voteDownLink in voteDownLinks) {
    voteDownLink.onclick = function(){ vote (down, this.parentNode)};
}

Also, the voting links store information of which type of vote has been made in the class HTML attribute. class="{{ vote is defined and vote == -1 ? 'downvoted' }}"

I think is not 100% bad. But according to the HTML 5.1 recommendation:

There are no additional restrictions on the tokens authors can use in the class attribute, but authors are encouraged to use values that describe the nature of the content, rather than values that describe the desired presentation of the content.

We are storing voting information, so maybe it is more appropriate to use data attributes for that surpose (https://developer.mozilla.org/en-US/docs/Learn/HTML/Howto/Use_data_attributes). Definition of data attributes from HTML 5.1 Recommendation:

Custom data attributes are intended to store custom data private to the page or application, for which there are no more appropriate attributes or elements.

This is just I think is the best approach. Please feel free to discuss it and correct me if I made any mistake.

There is some inline JavaScript in the voting mechanism: `onclick="vote ('down', this.parentNode)"`, which prevents the voting system to work with LibreJS and also doesn't separate the HTML from the JS. That code would be better in `freepost.js`, assigning the event onclick for each element using a for loop. Using something like this (I haven't tested it, it just an idea): ``` var voteUpLinks = document.getElementsByClassName('vote-up'); for (let voteUpLink in voteUpLinks) { voteUpLink.onclick = function(){ vote (up, this.parentNode)}; } var voteDownLinks = document.getElementsByClassName('vote-down'); for (let voteDownLink in voteDownLinks) { voteDownLink.onclick = function(){ vote (down, this.parentNode)}; } ``` Also, the voting links store information of which type of vote has been made in the `class` HTML attribute. ```class="{{ vote is defined and vote == -1 ? 'downvoted' }}"``` I think is not 100% bad. But [according to the HTML 5.1 recommendation](https://www.w3.org/TR/html51/dom.html#classes): > There are no additional restrictions on the tokens authors can use in the class attribute, but authors are encouraged to use values that **describe the nature of the content, rather than values that describe the desired presentation of the content**. We are storing voting information, so maybe it is more appropriate to use data attributes for that surpose (https://developer.mozilla.org/en-US/docs/Learn/HTML/Howto/Use_data_attributes). [Definition of data attributes from HTML 5.1 Recommendation](https://www.w3.org/TR/html51/dom.html#embedding-custom-non-visible-data-with-the-data-attributes): > Custom data attributes are intended to store custom data private to the page or application, for which there are no more appropriate attributes or elements. This is just I think is the best approach. Please feel free to discuss it and correct me if I made any mistake.
zPlus kommentierte vor 7 Jahren
Besitzer

We are storing voting information, so maybe it is more appropriate to use data attributes for that surpose

It's not used to store information. The upvoted, downvoted classes are used to draw green/red circles around the buttons. And the only purpose of the javascript is to change these classes (from green to red for example) when a new vote is cast. I don't think there's anything to change about these classes; they are not used to store information but to draw the colored circles.

> We are storing voting information, so maybe it is more appropriate to use data attributes for that surpose It's not used to store information. The `upvoted`, `downvoted` classes are used to draw green/red circles around the buttons. And the only purpose of the javascript is to change these classes (from green to red for example) when a new vote is cast. I don't think there's anything to change about these classes; they are not used to store information but to draw the colored circles.
zPlus kommentierte vor 7 Jahren
Besitzer

This should be fixed now. Please feel free to open another issue if you see any problem with the voting system :)

This should be fixed now. Please feel free to open another issue if you see any problem with the voting system :)
zPlus hat dieses Issue vor 7 Jahren aus einem Commit referenziert
zPlus hat vor 7 Jahren geschlossen
Anmelden, um an der Diskussion teilzunehmen.
Kein Meilenstein
Niemand zuständig
2 Beteiligte
Laden…
Abbrechen
Speichern
Hier gibt es bis jetzt noch keinen Inhalt.