Skip to content

Week one homework #1

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.

Sign up for GitHub

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

Merged
Catsudemo merged 1 commit into from
Aug 4, 2019
Merged

Week one homework #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
54 changes: 34 additions & 20 deletions homework/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,37 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<meta
name="viewport"
content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no"
/>
<meta name="theme-color" content="#000000" />
<meta name="apple-mobile-web-app-capable" content="yes" />
<meta name="mobile-web-app-capable" content="yes" />
<meta name="format-detection" content="telephone=no" />
<link rel="apple-touch-icon" href="./hyf.png" />
<link rel="shortcut icon" type="image/png" href="./hyf.png" />
<title>HYF-GITHUB</title>
<link href="https://fonts.googleapis.com/css?family=Roboto:400,700" rel="stylesheet" />
<link rel="stylesheet" href="./newStyle.css" />
</head>

<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no" />
<meta name="theme-color" content="#000000">
<meta name="apple-mobile-web-app-capable" content="yes">
<meta name="mobile-web-app-capable" content="yes">
<meta name="format-detection" content="telephone=no">
<link rel="apple-touch-icon" href="./hyf.png">
<link rel="shortcut icon" type="image/png" href="./hyf.png" />
<title>HYF-GITHUB</title>
<link href="https://fonts.googleapis.com/css?family=Roboto:400,700" rel="stylesheet">
<link rel="stylesheet" href="./style.css">
</head>

<body>
<div id="root"></div>
<script src="./index.js"></script>
</body>

</html>
<body>
<div class="header" id="selector_header">
<form name="chooseRepo">
<select onchange class="selector" id="repoSelect" name="repoSelect"> </select>
</form>
</div>
<div class="flex-wrapper">
<div id="repo">
Repo facts
</div>
<div id="contribDiv">
Contributors
</div>
</div>
<div id="root"></div>
<script src="./index.js"></script>
</body>
</html>
173 changes: 132 additions & 41 deletions homework/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,47 +1,138 @@
'use strict';

{
function fetchJSON(url, cb) {
const xhr = new XMLHttpRequest();
xhr.open('GET', url);
xhr.responseType = 'json';
xhr.onload = () => {
if (xhr.status < 400) {
cb(null, xhr.response);
} else {
cb(new Error(`Network error: ${xhr.status} - ${xhr.statusText}`));
}
};
xhr.onerror = () => cb(new Error('Network request failed'));
xhr.send();
}

function createAndAppend(name, parent, options = {}) {
const elem = document.createElement(name);
parent.appendChild(elem);
Object.keys(options).forEach(key => {
const value = options[key];
if (key === 'text') {
elem.textContent = value;
} else {
elem.setAttribute(key, value);
}
});
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' });
} else {
createAndAppend('pre', root, { text: JSON.stringify(data, null, 2) });
}
const repoSelector = document.querySelector('#repoSelect');

@mattiaslundberg mattiaslundberg Aug 21, 2019

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

From what I can see these variables aren't used, it's always a good idea to remove stuff that you don't need. Keeping them around will likely confuse you the next time you look at the code.

Neat freak

const contribDiv = document.querySelector('#contribDiv');
const repoDiv = document.querySelector('#repo');

function fetchJSON(url, cb) {
const xhr = new XMLHttpRequest();
xhr.open('GET', url);
xhr.setRequestHeader = 'Catsudemo https://api.github.com';
xhr.responseType = 'json';
xhr.onload = () => {
if (xhr.status < 400) {
cb(null, xhr.response);
} else {
cb(new Error(`Network error: ${xhr.status} - ${xhr.statusText}`));
}
};
xhr.onerror = () => cb(new Error('Network request failed'));
xhr.send();
}

function createAndAppend(name, parent, options = {}) {
const elem = document.createElement(name);
parent.appendChild(elem);
Object.keys(options).forEach(key => {
const value = options[key];
if (key === 'text') {
elem.textContent = value;
} else {
elem.setAttribute(key, value);
}
});
return elem;
}

function initPage(data, repoSelector) {
buildSelect(data, repoSelector);
}

function buildSelect(data, repoSelector) {
data
.map(repo => repo.name)
.sort()
.forEach(name => {
createAndAppend('OPTION', repoSelector, { text: name, value: name });

@mattiaslundberg mattiaslundberg Aug 21, 2019

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It seems like you add options to the select box multiple times, see if you can figure out what's causing that!

});
}
}

const HYF_REPOS_URL = 'https://api.github.com/orgs/HackYourFuture/repos?per_page=100';
function main(url) {
fetchJSON(url, (err, data) => {
const root = document.getElementById('root');
if (err) {
createAndAppend('div', root, { text: err.message, class: 'alert-error' });
} else {
// createAndAppend('pre', root, { text: JSON.stringify(data, null, 2) });
// buildSelect(data,repoSelector)
initPage(data, repoSelector);
}
});
}

window.onload = () => main(HYF_REPOS_URL);
const HYF_REPOS_URL = 'https://api.github.com/orgs/HackYourFuture/repos?per_page=100';

function displayContrib(contributor, contributions, avatar) {
const eachPersonUl = document.createElement('ul');

@mattiaslundberg mattiaslundberg Aug 21, 2019

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I find this function and the displayRepo function hard to read, try to add some blank lines where it's possible to logically separate different things. For example:

          const eachPersonUl = document.createElement('ul');

          const contributorName = document.createElement('li');
	  contributorName.innerText = `Username: ${contributor}`;
	  eachPersonUl.appendChild(contributorName);

	  const eachContributions = document.createElement('li');
	  eachContributions.innerText = `Contributions: ${contributions}`;

@mattiaslundberg mattiaslundberg Aug 21, 2019

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Also, as the code provides the createAndAppend function, it's a good idea to actually use it. That would help with readability.

const contributorName = document.createElement('li');
contributorName.innerText = `Username: ${contributor}`;
eachPersonUl.appendChild(contributorName);
const eachContributions = document.createElement('li');
eachContributions.innerText = `Contributions: ${contributions}`;
eachPersonUl.appendChild(eachContributions);
contribDiv.appendChild(eachPersonUl);
const eachAvatar = document.createElement('IMG');
eachAvatar.innerhtml = `<img src=${avatar}'>`;

@mattiaslundberg mattiaslundberg Aug 21, 2019

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I find this row a bit strange (and it looks like it isn't working as expected). You can actually set the attribute like this (and I think it would work):

eachAvatar.setAttribute('src', avatar);

contribDiv.appendChild(eachAvatar);
}

function displayRepo(repository, description, forks, updated) {
const eachRepoUl = document.createElement('ul');
const eachName = document.createElement('li');
eachName.innerText = `Repository: ${repository}`;
eachRepoUl.appendChild(eachName);
const eachDescription = document.createElement('li');
eachDescription.innerText = `Contributions: ${description}`;
eachRepoUl.appendChild(eachDescription);
const eachForks = document.createElement('li');
eachForks.innerText = `Forks: ${forks}`;
eachRepoUl.appendChild(eachForks);
const eachUpdated = document.createElement('li');
eachUpdated.innerText = `Updated: ${updated}`;
eachRepoUl.appendChild(eachUpdated);
repoDiv.appendChild(eachRepoUl);
}

const selectElement = document.querySelector('.selector');

selectElement.addEventListener('change', event => {

@mattiaslundberg mattiaslundberg Aug 21, 2019

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It looks like you're doing this outside of the main function. This can cause problems down the line (but looks like it's working during my tests), as some things might not be ready before window.onload is called. Please give moving the code inside the main function a try!

const result = document.querySelector('.result');
let repo = document.getElementById('repoSelect').value;
let repoURL = `https://api.github.com/repos/HackYourFuture/${repo}`;
let contribURL = `https://api.github.com/repos/HackYourFuture/${repo}/contributors`;

main(HYF_REPOS_URL);

@mattiaslundberg mattiaslundberg Aug 21, 2019

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm not sure why you're calling main here, you should probably remove it.


fetchJSON(repoURL, (err, data) => {
const root = document.getElementById('root');
if (err) {
createAndAppend('div', root, { text: err.message, class: 'alert-error' });
} else {
repoDiv.innerHTML = '';
let repository = data.name;
let description = data.description;
let forks = data.forks;
let updated = data.updated_at;
displayRepo(repository, description, forks, updated);
}
console.log('Here is the repodata');
});

fetchJSON(contribURL, (err, data) => {
const root = document.getElementById('root');
if (err) {
createAndAppend('div', root, { text: err.message, class: 'alert-error' });
} else {
contribDiv.innerHTML = '';
data.forEach(object => {
let contributor = object.login;
let contributions = object.contributions;
let avatar = object.avatar_url;
displayContrib(contributor, contributions, avatar);
});
console.log('Here is the contribdata');
}
});
});

window.onload = () => main(HYF_REPOS_URL);
65 changes: 65 additions & 0 deletions homework/newStyle.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
.header {
background-color: #176ab0;
color: white;
text-align: center;
padding: 5px;
}

#main-content {

@mattiaslundberg mattiaslundberg Aug 21, 2019

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This isn't used, so it's best to remove it.

width: 350px;
float: left;
padding: 10px;
}

#footer {

@mattiaslundberg mattiaslundberg Aug 21, 2019

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is never used, so it's best to remove it.

background-color: #176ab0;
color: white;
clear: both;
text-align: center;
padding: 5px;
}

.flex-wrapper {
font-family: Abel;
background-color: grey;
text-align: left;
padding-top: 25px;
padding-bottom: 25px;
font-size: 20px;
display: flex;
flex-direction: row;
justify-content: center;
background-color: #2E2C2F;
color: #444;
}

#repo {

@mattiaslundberg mattiaslundberg Aug 21, 2019

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The contents of #repo and #contribDiv looks the same, is there some way to avoid repeating all the code?

background-color: whitesmoke;
border-color: #2E2C2F;
width: 40%;
border: 5px;
padding: 20px;
margin: 20px;
}

#contribDiv {
background-color: whitesmoke;
border-color: #2E2C2F;
width: 40%;
border: 5px;
padding: 20px;
margin: 20px;
}

@media screen and (max-width: 600px) {

@mattiaslundberg mattiaslundberg Aug 21, 2019

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice, looks good on mobile!

.flex-wrapper {
-webkit-flex-direction: column;
flex-direction: column;
}
#repo {
width: 80%;
}
#contribDiv {
width: 80%;
}
}