Skip to content

Week 7 Github Tracker Linnea Frisk#73

Closed
Neaa99 wants to merge 25 commits intoTechnigo:mainfrom
Neaa99:main
Closed

Week 7 Github Tracker Linnea Frisk#73
Neaa99 wants to merge 25 commits intoTechnigo:mainfrom
Neaa99:main

Conversation

@Neaa99
Copy link
Copy Markdown

@Neaa99 Neaa99 commented Feb 25, 2022

Copy link
Copy Markdown

@JustineZwiazek JustineZwiazek left a comment

Choose a reason for hiding this comment

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

I loved reviewing your code Linnea. JavaScript was super concise and a pleasure to read. Since we worked on the same project all of the functions were clear to me, but since they keep recommending us to leave a lot of comments, my only feedback would be: there could be more comments describing functions 🙂 Great job with the site, really nice styling - I look forward seeing more of your work!

const container = document.getElementById('container')
const header = document.getElementById('header')

const API_TOKEN = TOKEN || process.env.API_KEY;
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 know many people struggled with the API token, so well done for setting it up! I skipped it this time, but planning to come back to it when I have time.

.then ((data) => {
console.log(data)
header.innerHTML = `
<a class="img-link" target="_blank" href="https://www.linkedin.com/in/linnea-frisk-59a170206/"><img src="${data.avatar_url}" width="100px" alt="User image"></a>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is interesting, I need to look into target. Very cleanly written function otherwise.

<p><span>• Last push:</span> ${new Date(repo.pushed_at).toDateString()}</p>
<p><span>• Branch:</span> ${repo.default_branch}</p>
<p><span>• Main language:</span> ${repo.language}</p>
<p id="commit-${repo.name}"><span>• Number of commits:</span> </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.

Really nice that you added info on the main language and the title of the page.


const pulls = data.find((pull) => pull.user.login === repo.owner.login);

fetchCommits(pulls.commits_url, repo.name)
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 was scratching my head how you got number of commits on all projects, but I think you were the person doing the pull each time? Unless there is some hidden trick I don't see :)


}

.header img:hover {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This was a very nice touch!


/* ANIMATIONS */

@keyframes colorChange {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Yay! I used keyframes for the first time with this project, so much fun!


}

.repo-link:hover {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Going the extra mile with styling during a really tough project needs to be praised ⭐

@Neaa99 Neaa99 closed this Nov 3, 2025
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