Skip to content

project-github-tracker Frida Axelsson#83

Open
Nosslexa wants to merge 10 commits into
Technigo:mainfrom
Nosslexa:main
Open

project-github-tracker Frida Axelsson#83
Nosslexa wants to merge 10 commits into
Technigo:mainfrom
Nosslexa:main

Conversation

@Nosslexa

Copy link
Copy Markdown

No description provided.

@savannah-hayes savannah-hayes left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great job with your project, I am able to understand your code easily. Everything works as it should and nothing is broken. It seems to meet the blue level requirements from what I can see and it looks nice.

Just a small tip I would add is that your code could be tided up a little, with cleaning up the empty lines and removing console logs, and so on.. But overall great job on this weeks project you should be proud of the project you have built!

Comment thread code/chart.js
Comment on lines +6 to +16
const drawChart = (amount) => {
const labels = [
'Projects done',
'Technigo projects',
];

const data = {
labels: labels,
datasets: [{
label: 'My First dataset',
data: [amount, 19-amount],

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great job passing in the amount of projected you have completed in your drawChart function. Nicely done!

Comment thread code/script.js
Comment on lines +93 to +107















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 would suggest removing the empty lines you have here and the ones in your css file to keep your file clean.

Comment thread .gitignore
@@ -0,0 +1 @@
code/secret.js No newline at end of file

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Did you know you could add the .DS_Store file into your gitignore file as well so it doesn't get committed?

Comment thread code/chart.js
);
}

console.log('test') No newline at end of file

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 think you forgot to remove a console log here on line 37 :)

Comment thread code/index.html
Comment on lines +13 to +29
<header class="hero">
<div id="userDataWrapper" class="user-data-wrapper"></div>
<section class="img-wrapper" id="imgWrapper"> </section>
</header>

<main class="project-wrapper" id="projects"></main>

<!-- This will be used to draw the chart 👇 -->
<canvas id="chart"></canvas>
<section class="chart-box">
<divc class="chart-wrapper" style="position: relative;">
<canvas id="chart"></canvas>
</div>
</section>

<footer>
<p> images from unsplash.com</p>
</footer>

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 job keeping your html file minimal and great job using semantic html.

Comment thread code/index.html

<script src="./script.js"></script>
<script src="./chart.js"></script>
<script src="./secret.js"></script>

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 would suggest removing this script tag for the secret js file. I don't believe you have a secret js file?

Comment thread code/script.js
Comment on lines +11 to +16
const options = {
method: 'GET',
headers: {
Authorization: 'API_TOKEN'
}
}

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 would suggest removing this as well since, I mentioned earlier that you don't seem to have a secret file and I think you can remove this code from line 11-16, since you don't seem to be using it in your script file.

Comment thread code/script.js
Comment on lines +24 to +25
console.log(data)
console.log(data[0].owner)

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 think you forgot to remove some console logs here in this file as well :)

Comment thread code/script.js
const forkedRepos = data.filter((repo) => repo.fork && repo.name.startsWith("project-"))
console.log(forkedRepos)

getPullRequests(forkedRepos)

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 job passing on your forked projects into your getPullRequests function.

Comment thread code/style.css
Comment on lines +13 to +24
background-image: url(./img/susan-wilkinson-SjhL-Zrol6A-unsplash.jpg) ;
background-position: left bottom;
background-repeat: no-repeat;
background-size: cover;
background-attachment: fixed;
position: relative;
height: 300px;
width: 100%;
display: flex;
justify-content: center;
align-items: center;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great job utilizing flex box in your project!

@bostrombundock

Copy link
Copy Markdown

Frida, as written above. your code is easy to follow and to understand the functions.

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.

3 participants