Skip to content

Week 7 project - Nadia Lefebvre#78

Open
nadialefebvre wants to merge 28 commits intoTechnigo:mainfrom
nadialefebvre:main
Open

Week 7 project - Nadia Lefebvre#78
nadialefebvre wants to merge 28 commits intoTechnigo:mainfrom
nadialefebvre:main

Conversation

@nadialefebvre
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@jessand77 jessand77 left a comment

Choose a reason for hiding this comment

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

Good job, Nadia! Very well written and well structured code with good choices of variable names, function names etc and many comments explaining what the functions do and so on. Advanced code that could have been more difficult to understand but you made it easy to follow and to learn new things from it 🙂

<link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/5.12.0-2/css/all.min.css">
<link rel="stylesheet" href="./style.css" />
<script src="https://cdnjs.cloudflare.com/ajax/libs/Chart.js/3.7.1/chart.min.js"></script>
<link rel="icon" type="image/x-icon" href="./favicon.ico">
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 favicon!

</select>

<script src="./script.js"></script>
<!-- All repo cards are injected here by JavaScript -->
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good comment that explains what is happening here, makes it easier to follow.

Comment on lines +29 to +32
<div class="repos-chart">
<!-- Technigo projects chart is injected here by JavaScript -->
<canvas id="reposChart"></canvas>
</div>
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 it was a good choice to place the chart up here in the header. It gives the page layout a good balance.

Comment on lines +1 to +7
:root {
--white: #ffffff;
--lightbackground: #f9f7f7;
--lightblue: #dbe2ef;
--mediumblue: #3f72af;
--darkblue: #112d4e;
}
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 use of var()-colors makes the code easier to understand (and it's easier to change the colors if you would want to). Good!

// Function for the tab buttons in repo card
const openTab = (event, tabName) => {
let i
let repoContent = event.currentTarget.parentNode.parentNode.getElementsByClassName("repo-content") // targets the content on each individual repo card
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 don't think I understand all of this line right now but I really like the result 👍

Comment on lines +35 to +36
// Function to create the profile part of the header with user name and picture
// -------------------------------------------------- FETCH 1 --------------------------------------------------
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Very good use of comments here as well 😀

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