#47 Some issues with voting mechanism

Closed
opened 7 years ago by jorgesumle · 2 comments

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 commented 7 years ago
Owner

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 commented 7 years ago
Owner

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 referenced this issue from a commit 7 years ago
Sign in to join this conversation.
No Milestone
No assignee
2 Participants
Loading...
Cancel
Save
There is no content yet.