-
Notifications
You must be signed in to change notification settings - Fork 11
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
Add files via upload #6
Conversation
function createTweetOl(tweet) { | ||
var ol = document.createElement('ol'); | ||
var el = document.querySelector('div'); | ||
el.appendChild(ol); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two lines seem useless
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh wait no, they're not useless, but they're not what was asked for.
This function should return an <ol>
and the calling context should be the one deciding where the ol should be appended.
if (tweet.lang === "fr") { | ||
li.classList.add("fr"); | ||
} else { | ||
li.classList.add("en"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need that part, but if you really want it, prefer the lang
attribute.
@@ -0,0 +1,12 @@ | |||
"use strict"; | |||
|
|||
function createTweetOl(tweet) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function creates an <ol>
which only contains a single tweet.
It should take an array of tweets as argument and return an <ol>
with one <li>
per tweet
var el = document.querySelector('div'); | ||
|
||
tweets.forEach(function(tweet) { | ||
createTweetOl(tweet); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Related to above comment. There should not be one ol per tweet, but rather a single ol. Also, it's in the calling context that should be decided where the ol is appended.
<output></output> | ||
</body> | ||
<body> | ||
<button id="buttonTweets" >Display French Tweets</button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The body should remain empty. Create it in JavaScript.
|
||
document.getElementById('buttonTweets').addEventListener("click", function() { | ||
onlyFr = !onlyFr; | ||
var liFr = document.querySelectorAll(".en"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll take a longer time on this in class, but you shouldn't have to read information from the DOM. Instead, you should be re-creating new elements from the data (and only read the information to generate the elements from the data).
button.textContent = text; | ||
|
||
// Add new button to body | ||
body.appendChild(button); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function should only create the element, not append it to the body.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The calling context should be deciding where the element is appended.
var mouseDownTimeStamp, mouseUpTimeStamp, clickTime; | ||
|
||
Array.from(listButtonFat).forEach(function(button) { | ||
button.addEventListener("mouseup", function(result) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All these event listeners belong in the createClickTimeButton
function
********************** LISTENERS FAT BUTTON ********************** | ||
******************************************************************/ | ||
|
||
var listButtonFat = document.getElementsByClassName('buttonFat'), timer, num = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You shouldn't have to read from the DOM.
Corrige les différents points en commentaires que je puisse valider |
(d'ici la semaine prochaine) |
Rien n'est corrigé -_-# |
Exercice ECV