Skip to content
This repository was archived by the owner on May 14, 2024. It is now read-only.
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 85 additions & 21 deletions homework/src/index.js
Original file line number Diff line number Diff line change
@@ -1,47 +1,111 @@
'use strict';
"use strict";

function main(url) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You should not embed all functions inside one gigantic main() function. The skeleton code that was provided already had the correct structure.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is still not fixed.


const root = document.getElementById("root");
const header = createEl("header", root);
const article = createEl("article", root);
createEl("label", header, { txt: "HYF Repositories" });
const select = createEl("select", header);

{
function fetchJSON(url, cb) {
const xhr = new XMLHttpRequest();
xhr.open('GET', url);
xhr.responseType = 'json';
xhr.onload = () => {
if (xhr.status < 400) {
if (xhr.status === 200) {
Copy link
Contributor

Choose a reason for hiding this comment

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

By changing the condition of the if statement you are now considering any status code not equal to 200 as an error. This is an error by itself. Any code equal or greater than 400 is an error. All codes less than 400 are not errors. See: https://www.restapitutorial.com/httpstatuscodes.html

If you expect a 200 code but get some other code but still less than 400 you need to find some other way of dealing with. But you can't report it to the user as an HTTP error.

Copy link
Contributor

Choose a reason for hiding this comment

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

This still needs to be fixed.

cb(null, xhr.response);
} else {
cb(new Error(`Network error: ${xhr.status} - ${xhr.statusText}`));
cb(new Error(`Network Error: ${xhr.status} - ${xhr.statusText}`));
}
};
xhr.onerror = () => cb(new Error('Network request failed'));
xhr.onerror = () => {
cb(new Error(`Network request failed`));
};
xhr.send();
}

function createAndAppend(name, parent, options = {}) {
function createEl(name, parent, options = {}) {
const elem = document.createElement(name);
parent.appendChild(elem);
Object.keys(options).forEach((key) => {
const value = options[key];
if (key === 'text') {
elem.innerText = value;
for (let key in options) {
if (key === "txt") {
Copy link
Contributor

Choose a reason for hiding this comment

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

There is really no reason to use cryptic abbreviations. Why not just used text instead of txt or name instead of nm. Professionals write clean code, see for instance this link:

https://medium.com/mindorks/how-to-write-clean-code-lessons-learnt-from-the-clean-code-robert-c-martin-9ffc7aef870c

elem.innerText = options.txt;
} else {
elem.setAttribute(key, value);
elem.setAttribute(key, options[key]);
}
});
}
return elem;
}

function main(url) {
fetchJSON(url, (err, data) => {
const root = document.getElementById('root');
if (err) {
createAndAppend('div', root, { text: err.message, class: 'alert-error' });
fetchJSON(url, (err, repos) => {
if (err === null) {
repos.sort((a, b) => a.name.localeCompare(b.name))
.forEach((repo, i) => {
let nm = repo.name.charAt(0).toUpperCase() + repo.name.slice(1);
createEl("option", select, { txt: nm, value: i });
});
renderRepo(repos, select.value);
} else {
root.innerHTML = "";
createEl("div", root, { txt: err.message, id: "error" });
}
});

function renderRepo(repos, index) {
article.innerHTML = "";
repoDetails(repos[index]);
contributors(repos[index].contributors_url);

select.addEventListener("change", () => {
article.innerHTML = "";
repoDetails(repos[select.value]);
contributors(repos[select.value].contributors_url);
});

}

function repoDetails(details) {

const table = createEl("table", article);
let tr = createEl("tr", table);
createEl("th", tr, { txt: "Repository:" });
let td = createEl("td", tr);
createEl("a", td, {
txt: details.name, href: details.html_url, target: "_blank"
});

if (details.description) {
createEl("tr", table).innerHTML =
`<th>Description:</th><td>${details.description}</td>`;
}
createEl("tr", table).innerHTML =
`<th>Forks:</th><td>${details.forks_count}</td>`;
createEl("tr", table).innerHTML =
`<th>Updated:</th><td>${details.updated_at}</td>`;
}

function contributors(url) {
fetchJSON(url, (err, infos) => {
if (err === null) {
const ul = createEl("ul", article);
infos.forEach(info => {
let li = createEl("li", ul);
let a = createEl("a", li, {
href: info.html_url, target: "_blank"
});
createEl("img", a, { src: info.avatar_url });
createEl("p", a, { txt: info.login });
createEl("span", a, { txt: info.contributions });
});
} else {
createAndAppend('pre', root, { text: JSON.stringify(data, null, 2) });
article.innerHTML = "";
createEl("div", article, { txt: err.message, id: "error" });
}
});
}

const HYF_REPOS_URL = 'https://api.github.com/orgs/HackYourFuture/repos?per_page=100';

window.onload = () => main(HYF_REPOS_URL);
}

const HYF_REPOS_URL = 'https://api.github.com/orgs/HackYourFuture/repos?per_page=100';
window.onload = () => main(HYF_REPOS_URL);
102 changes: 99 additions & 3 deletions homework/src/style.css
Original file line number Diff line number Diff line change
@@ -1,3 +1,99 @@
.alert-error {
color: red;
}
@import url('https://fonts.googleapis.com/css?family=Roboto');
body {
margin: 0;
}
#error {
background-color: #f8d7da;
color: #7b1c2a;
width: 100%;
padding: 12px 20px;
border-radius: 6px;
}
#root {
max-width: 70%;
margin: auto;
font-family: 'Roboto', Arial, sans-serif;
}
header {
width: 100%;
background-color: #3f51b5;
}
header label {
display: inline-block;
margin: 17px;
font-size: 20px;
color: #fff;
}
header select {
display: inline-block;
padding: 6px;
}

article {
display: flex;
flex-flow: wrap;
padding: 15px 0;
font-size: 15px;
}
article table {
flex: 5;
padding: 20px;
margin: 0 5px 0 0;
box-shadow: 0px 0px 8px rgb(148, 148, 148);
}
article table tr th {
text-align: left;
}
article table tr td {
padding: 0 0 0 8px;
}
article ul {
flex: 5;
list-style-type: none;
padding: 0;
margin: 0 0 0 5px;
box-shadow: 0px 0px 8px rgb(148, 148, 148);
}
article ul p {
margin: 0;
padding: 15px;
font-size: 14px;
color: rgb(95, 94, 94);
}
article ul li a {
display: flex;
align-items: center;
text-decoration: none;
margin: 0;
padding: 15px;
}
article ul li a img {
width: 45px;
height: 45px;
border-radius: 6px;
}
article ul li a p {
flex: 8;
margin: 0;
padding: 0 0 0 15px;
}
article ul li a span {
margin: 0;
padding: 1px 6px;
background-color: #808080;
border-radius: 4px;
font-size: 14px;
color: #fff;
}

@media screen and (max-width: 750px) {
#root {
max-width: 100%;
}

article table, article ul {
flex: 100%;
width: 100%;
margin: 0 0 10px 0;
}
}