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
93 changes: 93 additions & 0 deletions Week1/github.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
'use strict';

const URL_Rep = 'https://api.github.com/orgs/HackYourFuture/repos?per_page=100';
Copy link
Contributor

Choose a reason for hiding this comment

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

From your indentation I suspect that you intended to create a block scope here to prevent your function from polluting the global scope. However, to do so, you must use curly braces, i.e.:

'use strict';
{
  const URL_Rep = ...
  //...

function renderRepository(container, repository) {
let dateOfRepository = new Date(repository.updated_at);
Copy link
Contributor

Choose a reason for hiding this comment

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

Because you created your homework in a different folder than was specified in the homework assignment, your code is not checked by ESLint (you are missing an .eslintrc file in the Week1 folder where you placed your homework). These are the warnings and errors that ESLint produced:

/Users/jimcramer/hackyourfuture/class16/genegr88/Week1/github.js
   4:9   warning  Identifier 'URL_Rep' is not in camel case                    camelcase
   6:9   warning  'dateOfRepository' is never reassigned. Use 'const' instead  prefer-const
  13:90  warning  Missing semicolon                                            semi
  59:9   warning  Missing semicolon                                            semi
  69:94  warning  Missing semicolon                                            semi
  72:14  warning  'err' is defined but never used                              no-unused-vars
  73:31  warning  'error' is not defined                                       no-undef
  86:72  warning  Missing semicolon                                            semi
  87:89  warning  Missing semicolon                                            semi
  91:70  warning  Missing semicolon                                            semi
  92:9   warning  Missing semicolon                                            semi
  95:2   error    Newline required at end of file but not found                eol-last

✖ 12 problems (1 error, 11 warnings)

container.innerHTML = "";
const innerTable = createAndAppend("table", container);
const tBody = createAndAppend("tbody", innerTable);
const tr = createAndAppend('tr', tBody);
createAndAppend('td', tr, {text: "Repository:", class: "nameRow"});
const tdForLink = createAndAppend('td', tr,);
createAndAppend('a', tdForLink, {text: repository.name, href: repository.html_url})
const tr1 = createAndAppend('tr', tBody);
createAndAppend('td', tr1, {text: "Description:", class: "nameRow"});
createAndAppend('td', tr1, {text: repository.description});
const tr2 = createAndAppend('tr', tBody);
createAndAppend('td', tr2, {text: "Forks:", class: "nameRow"});
createAndAppend('td', tr2, {text: repository.forks});
const tr3 = createAndAppend('tr', tBody);
createAndAppend('td', tr3, {text: "Updated:", class: "nameRow"});
createAndAppend('td', tr3, {text: dateOfRepository.toLocaleString()});


}
Copy link
Contributor

Choose a reason for hiding this comment

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

The preceding two blank lines are meaningless and should be removed.

Add a single blank line between function definitions.

//contributors box
function renderContributors(containerForContributors, repository) {
containerForContributors.innerHTML = "";
Copy link
Contributor

Choose a reason for hiding this comment

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

You have a mix of single and double quotes for string delimiters. Choose one and then use it consistently throughout your code.

const contributorsURL = repository['contributors_url'];
Copy link
Contributor

Choose a reason for hiding this comment

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

Use dot notation to access object properties if you know the property name beforehand:

const contributorsURL = repository.contributors_url;

This gives IDE's such as VSCode and linters such ESLint a better chance to analyze your code.

Use square brackets only if the property name is "computed", e.g.

const fullName = `${firstName} ${lastName}`;
const person = people[fullName];

createAndAppend('p', containerForContributors, { text: 'Contributions' });
fetchJSON(contributorsURL)
.catch(err => {
createAndAppend('div', containerForContributors, { text: err.message, class: 'alert-error' });
})

.then(contributors => {
Copy link
Contributor

Choose a reason for hiding this comment

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

The .catch() should be at the end of the promise chain. Currently your error handling does not work correctly because the .catch() method returns a new promise that resolves to undefined (because you are not returning anything in the function body of your catch handler). Next, you chain a .then on that new promise which expects a contributors array but instead gets undefined. While it might look okay in the UI, the code is wrong.

const contributorsTable = createAndAppend('table', containerForContributors, { id: 'contributorsTable' });

const contributorsTbody = createAndAppend('tbody', contributorsTable, { id: 'contributorsTbody' });

contributors.forEach(contributor => {

const contLink = contributor.html_url;
const contImg = contributor.avatar_url;
const contName = contributor.login;
const contNum = contributor.contributions;
const contributorsLink = createAndAppend('a', contributorsTbody, { href: contLink, target: '_blank', class: 'contributorsLink' });

const contributorsTr = createAndAppend('tr', contributorsLink, { id: `${contributor['id']}`, class: 'contributorsTr' });

const tdImg = createAndAppend('td', contributorsTr, { id: `${contName}Td`, class: 'inform-contrib' });

createAndAppend('img', tdImg, { src: contImg, alt: `picture of ${contName}`, class: 'contributor-images' });

createAndAppend('td', contributorsTr, { text: contName, class: 'inform-contrib contributor-name' });

createAndAppend('td', contributorsTr, { text: contNum, class: 'inform-contrib contributor-number' });
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Below is the HTML that your code generates for the contributors table. If you look at the documentation for tbody you will find that the only children allowed at tr elements.

<table id="contributorsTable">
  <tbody id="contributorsTbody">
    <a href="https://github.com/gijscor" target="_blank" class="contributorsLink">
      <tr id="15912395" class="contributorsTr">
        <td id="gijscorTd" class="inform-contrib">
          <img src="https://avatars0.githubusercontent.com/u/15912395?v=4" alt="picture of gijscor" class="contributor-images">
        </td>
        <td class="inform-contrib contributor-name">gijscor</td>
        <td class="inform-contrib contributor-number">12</td>
      </tr>
    </a>
    <a href="https://github.com/wouterkleijn" target="_blank" class="contributorsLink">
      <tr id="37488847" class="contributorsTr">
        <td id="wouterkleijnTd" class="inform-contrib">
          <img src="https://avatars1.githubusercontent.com/u/37488847?v=4" alt="picture of wouterkleijn" class="contributor-images">
        </td>
        <td class="inform-contrib contributor-name">wouterkleijn</td>
        <td class="inform-contrib contributor-number">4</td>
      </tr>
    </a>
  </tbody>
</table>

})
}


function main(url) {
const root = document.getElementById('root');
const divForSelect = createAndAppend('div', root, {id: "container-select"});
createAndAppend('h5', divForSelect, {text: "HYF Repositories", class: "heading"});
const select = createAndAppend('select', divForSelect, {id: "menu"});
const container = createAndAppend('div', root, {id: "first-container"});
const containerForContributors = createAndAppend('div', root, {id: "second-container"})

fetchJSON(url)
.catch(err => {
container.innerHTML = error.message;
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. The .catch() should be at the end of the promise chain.
  2. The argument is called err but you are trying to access error.

I assume you haven't actually tried to force an error as the assignment asked, otherwise you would have caught this error.

return;
})

.then(repositories => {

repositories.sort((a, b) => a.name.localeCompare(b.name));

repositories.forEach((repository, index) => {
createAndAppend('option', select, { text: repository.name, value: index });
});

select.addEventListener('change', (event) => {
renderRepository(container, repositories[event.target.value])
renderContributors(containerForContributors, repositories[event.target.value])
});
Copy link
Contributor

Choose a reason for hiding this comment

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

You will notice that there is a slight delay between showing the repo info and the contributors info. Time-wise it looks like this.

[show repo][clear contributors container][fetch contributors].......[show contributors]

You can improve on the user experience by postponing the (re-)rendering until the contributor data has been returned:

[fetch contributors].......[clear contributors container][render contributors][show repo]

Now, while the contributors are being fetched the screen remains unchanged.

To achieve this in your code, swap the two lines as follows:

renderContributors(containerForContributors, repositories[event.target.value]);
renderRepository(container, repositories[event.target.value]);

Then, in your renderContributors() function, postpone clearing out the containerForContributors until after you have received the response from the network:

function renderContributors(containerForContributors, repository) {
  const contributorsURL = repository['contributors_url'];
  createAndAppend('p', containerForContributors, { text: 'Contributions' });
  fetchJSON(contributorsURL)
    .then(contributors => {
      containerForContributors.innerHTML = "";
     //...


renderRepository(container, repositories[0]);
renderContributors(containerForContributors, repositories[0])
})
}
window.onload = () => main(URL_Rep);
15 changes: 15 additions & 0 deletions Week1/index2.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<meta http-equiv="X-UA-Compatible" content="ie=edge">
<link rel="stylesheet" href="style2.css">
<title>Application for repositories</title>
</head>
<body>
<div id="root"></div>
<script src="util.js"></script>
<script src="github.js"></script>
</body>
</html>
119 changes: 119 additions & 0 deletions Week1/style2.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
body {
font-family: Arial, Helvetica, sans-serif;
background-color: rgb(232, 232, 236);
min-width: 330px;
box-sizing: border-box;
display: flex;
justify-content: center;
}
#root {
width: 100%
}
* {
box-sizing: border-box;
}
h5 {
padding: 0;
margin: 0;
}
#container-select {
color: white;
background-color: rgb(34, 51, 150);
padding: 20px 16px;
margin-bottom: 16px;
display: flex;
flex-direction: row;
align-items: center;
}
#menu {
margin-left: 15px;
font-size: 14px;
width: 270px;
height: 32px;
padding: 5px;
}
#root {
max-width: 800px;
margin: 0 auto;
}
#first-container {
padding: 20px;
border-radius: 5px;
background-color: #fff;
box-shadow: 5px 5px 8px ;
margin-bottom: 15px;
}
table {
border-collapse: collapse;
}
td {
padding-left: 15px;
}
td.nameRow {
font-weight: bold;
}
#second-container {
margin: 5px;
background-color: rgb(241, 241, 243);
border: 2px rgb(241, 241, 243) solid;
border-radius: 7px;
box-shadow: 1px 2px 5px 1px #888888;
background-color: rgb(208, 245, 245);
}
#second-container p {
margin: 20px 10px 20px 10px;
color: black;
font-size: 0.8em;
}
#contributorsTable {
width: 100%;
}
#second-container a:link {
text-decoration: none;
color: inherit;
}
#second-container a:visited {
text-decoration: none;
color: inherit;
}
#second-container a:nth-last-child(n+2) tr {
border-bottom: 1.2px solid rgb(232, 232, 236);
}
.contributorsTr {
display: flex;
align-items: center;
justify-content: space-between;
}
.contributor-images {
width: 60px;
border-radius: 6px;
margin: 10px;
}
.contributor-name {
margin-right: auto;
}
.contributor-number {
display: inline;
background-color: rgb(111, 111, 111);
min-width: 25px;
border-radius: 5px;
color: rgb(232, 232, 236);
padding: 5px;
text-align: center;
margin-right: 5px;
}
@media only screen and (min-width:700px) {
#root {
max-width: 800px;
}

#first-container {
width: calc(60% - 10px);
float:left;
}

#second-container {
width: calc(40% - 15px);
float: right;
}
}
35 changes: 35 additions & 0 deletions Week1/util.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
'use strict';
{
function fetchJSON(url) {
return new Promise((resolve,reject) => {
const xhr = new XMLHttpRequest();
xhr.open("GET", url);
xhr.responseType = 'json';
xhr.onload = () => {
if (xhr.status < 400) {
resolve(xhr.response)
} else {
reject(new Error(`Network error: ${xhr.status} - ${xhr.statusText}`));
}
}
xhr.onerror = ()=> {
reject(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 => {
if (key === "text") {
elem.innerHTML = options[key];
} else {
elem.setAttribute(key, options[key]);
}
})
return elem;
}
window.fetchJSON = fetchJSON;
window.createAndAppend = createAndAppend;
}