#47 Some issues with voting mechanism

Закриті
7 роки тому відкрито jorgesumle · 2 коментарів
Jorge Maldonado Ventura відкоментовано 7 роки тому

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 відкоментовано 7 роки тому
Власник

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 відкоментовано 7 роки тому
Власник

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 згадано цю проблему в коміті 7 роки тому
zPlus закрито 7 роки тому
Підпишіться щоб приєднатися до обговорення.
Без Мітки
Етап відсутній
Немає відповідального
2 учасників
Завантажується...
Скасувати
Зберегти
Тут ще немає жодного вмісту.