Skip to content
This repository was archived by the owner on May 14, 2024. It is now read-only.

Conversation

@Dumie1
Copy link

@Dumie1 Dumie1 commented Jun 12, 2018

Added javascript3 homework week1. Not really confident with my index.js file or code...

Copy link

@harditsingh harditsingh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job Dumie! Your code looks pretty good, but some things can be improved. Cheers!


function repoData(error, data) {
if (error !== null) {
console.error(error);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should not print the error to the console as the user would never know what went wrong. Instead, you should show the error on the webpage

const root = document.getElementById('root');

root.innerHTML = null;
fetchJSON("https://api.github.com/orgs/HackYourFuture/repos?per_page=100", repoData);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should not use URLs like this, you should put them in a variable instead. Without a variable, it can get confusing when you try to reuse the code or try to change something

for (const obj of arrayOfObjects) {
const optionItem = createAndAppend('option', select);
optionItem.innerText = obj.name;
optionItem.value = 'https://api.github.com/repos/HackYourFuture/' + obj.name;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You forgot to sort the elements of the drop-down menu

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants