Skip to content
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.

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

Week nr 7 github tracker #113

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Week nr 7 github tracker #113

wants to merge 13 commits into from

Conversation

Kyparn
Copy link

@Kyparn Kyparn commented Feb 27, 2022

This was a heavy one :)

Copy link

@JenFi72 JenFi72 left a comment

Choose a reason for hiding this comment

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

Very good project Simon! The page looks good in different responsive "moods" and you have clearly reached all requirement in order to meet blue level requirements for this task:

  • A list of all repos that are forked from Technigo
  • Your username and profile picture
  • Most recent update (push) for each repo
  • Name of your default branch for each repo
  • URL to the actual GitHub repo
  • Number of commits for each repo
  • It should be responsive (mobile first)
  • A visualisation, for example through a pie chart, of how many projects you've done so far, compared to how many you will do (in total it will be 19 weekly projects

Concerning the code it is structured, easy to follow and clean. Very good job!

Comment on lines +5 to +21
const drawChart = (amount) => {
const configchart = {
type: 'doughnut',
data: {
labels: ['Complete', 'Not Complete'],
datasets: [
{
label: 'Technigo projects rate ',
data: [amount, 19 - amount],
backgroundColor: ['#77E327', '#E33527'],
hoverOffset: 3,
},
],
},
}
const myProjects = new Chart(ctx, configchart)
}
Copy link

Choose a reason for hiding this comment

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

Clean and easy to follow :)

<script src="./script.js"></script>
</body>
</html>
Copy link

Choose a reason for hiding this comment

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

This is a rather short and easy-to follow HTML without unnecessary divs etc, though it would be nice to add comments to declare/describe different sections - like "//PROFILE SECTIONS", "PROJECT SECTION"

}

//Prints out the user info in the HTML

Copy link

Choose a reason for hiding this comment

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

Nice comments, makes it easy to follow.

<div class="repoInfo" id="${projectID}">
<img src="github.png" class="repoimg" alt="logo" width="25px" />
<p class="cardInfo">Project name:
${repo.name.replace('project-', '').replace('-', ' ')}
Copy link

Choose a reason for hiding this comment

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

Neat code string, didn't think of this method myself :)

}
// Function to get commits and print them out
const getCommits = (projects, projectID) => {
const GIT_COMMIT_API = projects.commits_url.replace('{/sha}', '')
Copy link

Choose a reason for hiding this comment

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

Nice way to add commits id :)

Comment on lines +66 to +73
@keyframes color-change {
0% {
background-color: rgb(7, 41, 152);
}
10% {
background-color: rgb(65, 32, 153);
}
Copy link

Choose a reason for hiding this comment

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

Maybe add some comments here as well to make it easier t follow.

Comment on lines +102 to +119
.repoInfo {
justify-items: center;
display: grid;
}

.personData {
justify-items: center;
display: grid;
}
.projects {
grid-template-columns: 1fr 1fr;
}
.cardInfo {
font-size: 16px;
padding: 5px;
}
}
Copy link

Choose a reason for hiding this comment

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

Good that you are being thorough with mediaqueries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants