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

Conversation

@ThreePalmTrees
Copy link

@ThreePalmTrees ThreePalmTrees commented Jun 10, 2018

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.

Amazing job Husam! The website and the code look great! I have a few concerns though, please check the comments below.

return response;

} else if (xhr.status === 404) {
console.log("Error: resource not found");

Choose a reason for hiding this comment

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

The assignment says that you have to display the error on the screen, printing it to console isn't effective enough as people don't usually check the console log.

let header = createAndAppend("header", root)
let selectTag = createAndAppend("select", header);

let wrapper = createAndAppend("div", root);

Choose a reason for hiding this comment

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

While the code looks amazing, this should have been wrapped in a main() function.

objects.forEach(object => {
let optionTag = createAndAppend("option", selectTag);
optionTag.value = object.name;
optionTag.textContent = object.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 options of the drop-down menu in an alphabetical order.

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