Skip to content

GitHub tracker Joanna Ringqvist#94

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

GitHub tracker Joanna Ringqvist#94
joannaringqvist wants to merge 10 commits into
Technigo:mainfrom
joannaringqvist:main

Conversation

@joannaringqvist

Copy link
Copy Markdown

No description provided.

@EmmaaBerg EmmaaBerg 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.

A really good job! A well-structured page and code that was easy to understand. It follows the guidelines for how to write good code as well. It fulfills the blue requirements and works fine in the responsive aspect. I liked the way you presented your projects in a table and how it is responsive in itself on smaller screens.

Comment thread code/index.html

<!-- This will be used to draw the chart 👇 -->
<canvas id="chart"></canvas>
<header>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Overall a clean and structured HTML and good with the semantic HTML-elements

Comment thread code/index.html
<h1 class="heading">GitHub Tracker for:</h1>
<div class="user">
<p class="username" id="username"></p>
<img id="picture" class="picture" />

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

For images I think the alt -attribute is good to add if the image can't be displayed. But I'm not sure if it should be included here or in the JS where the fetch is?

Comment thread code/index.html

<script src="./script.js"></script>
<main>
<table class="projects">

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Really liked the way you presented the data in a table!

Comment thread code/index.html
</main>
<canvas id="chart" class="chart">
</canvas>
<p id="numberOfProjects" class="number-of-projects"></p>

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 naming conventions for JS and CSS

Comment thread code/script.js
Comment on lines +34 to +35
<td id="commits-${repo.name}"></td>
<td><a class="repo-url" href="${repo.html_url}">${repo.html_url}</a></td>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

A really small comment, but here is the only time double quotation marks are used. To make the code consistent only single quotation marks (or double) should be used.

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