zaherqasqas/hyf-javascript03

Javascript3 homework

hyf-javascript03's Issues

Feedback homework for week 3

Hi Zaher, here is my feedback on your homework for week 3.

Your application loads and runs OK. It is somewhat responsive, although the use of screen space on a mobile phone leaves to be desired (see image):

Your code looks well organised and well formatted. You have correctly implemented promises and async/await.

1. You didn't implement ES6 classes, but don't worry about it. It will be covered again in the React module, which is still some time away.

2. You did not sort the list repos displayed in the select box as was asked in Step 4 of the homework.

3. You do render (network) error messages to the web page, but they could be more user-friendly. Currently only the message 'Not found' is displayed, which is a bit cryptic for the average user.

Bottom line: I know you struggle with learning all this stuff and need to work hard to get results. Still, if you can keep this up and continue to ask for help when needed I think you have a good chance to make it through the rest of the curriculum.

Feedback homework week 1

Hi Zaher,

Here is my feedback on your week 1 homework.

Despite the troubles you have gone through to produce this result, I'm actually quite happy with what I see here. Your application meets the requirements of the assignment. Of course, your UI could benefit from some CSS styling. Maybe you can improve on that in the homework of week 2 and 3.

Here are my detail comments:

1. When your application is first loaded, it has the "Angular" repo in the <select> element, but the Angular repo information is not displayed. This is because you rely on the <select> element's 'change' event to update the repo info in the DOM. However, when you initialize the <select> element with <options> elements, that 'change' event is not fired. The solution would be to call your renderInfo() function manually, but since that function expects an Event object that would be difficult. Therefore it would be better if that function would take a repo name as its argument, as shown here:

   function reposInfo(repoName) {
     const urlCon = 'https://api.github.com/repos/HackYourFuture/' + repoName + '/contributors';
     const repoUrl = 'https://api.github.com/repos/HackYourFuture/' + repoName;
     fetchJSON(urlCon, list_render_con);
     fetchJSON(repoUrl, reop_info);
   }

   Of course, you would need to modify the call to addEventListener() too. We could use a fat arrow function for that:

   select.addEventListener("change", () => reposInfo(select.value));

   Now, you can call reposInfo() manually at the end of your renderRepos() function:

   function renderRepos(repos) {
     const root = document.getElementById('root');
     const select = createAndAppend('select', root);
     const repoContainer = createAndAppend('div', root);
     repoContainer.id = 'repo-container';
     const contrContainer = createAndAppend('div', root);
     contrContainer.id = 'contribs-container';
     select.addEventListener("change", () => reposInfo(select.value));
     repos.forEach(repo => {
       const option = createAndAppend('option', select);
       option.innerHTML = repo.name;
       option.setAttribute('value', repo.name)
     });
     reposInfo(select.value); // added
   }

2. Your code consists of a number of function definitions, and somewhere, hidden between those definitions you actually execute some code. It would be better to move the code you execute immediately to a separate function. I often use a function called main() for that purpose, that I usually locate at the end of the file. In your case that function main() could contain:

   function main() {
     function callback(error, data) {
       if (error !== null) {
         // console.error(error);
       } else {
         renderRepos(data);
       }
     }
     fetchJSON(url, callback);
   }

   You can further simplify that function by using an ES6 fat arrow function for the callback (they are not that hard to understand as you might think):

   function main() {
     fetchJSON(url, (error, data) => {
       if (error !== null) {
         // console.error(error);
       } else {
         renderRepos(data);
       }
     });
   }

   Finally, you can execute your function main() by assigning it to window.onload. The onload event handler is called by the browser when it has completely loaded the web page and all of its resources.

   window.onload = main;

3. Please do not use names with underscores inside. The convention in JavaScript it to use camelCase names. Please checkout the HYF fundamental on Naming Conventions. Also, use abbreviations only if they are generally well understood. For instance, where you use contrContainer I would prefer contributorContainer. You have to type it only once. The next time you want to use it, VSCode will show that name in its popup suggestions list, as soon as you type the first few letters.

4. Please make sure that you follow up on any warnings (green and red wavy underlines in VSCode) you get from the VSCode spelling checker and ESLint . To let ESLint do its job you must include an .eslintrc file in the main folder of your repo. It's not there now. Create that file and paste in the contents that I posted in slack during the last lecture. As for (English) spelling errors, even if your English is not 100%, nobody looking at your code needs to know that if you just follow up on spelling error warnings. Google or use an English dictionary if your are unsure about a particular spelling (I do it frequently).

5. Formatting: Leave a blank line between function definitions. Inside function definitions, use a single blank line between blocks of related code if you think that improves readability. Do not carelessly put or leave in blank lines if they serve no obvious purpose. Also, remove all commented-out code (in your case mostly console.log statements). Once they have served their purpose they just become a distraction when you (or I) review your code. It's not a big deal to re-add a console.log statement later, should that be needed.

With this, I have no real further comments to make. In week 2 and week 3 will continue working and improving on this app; lots of opportunity for further fine-tuning and polishing up.

Well done!