project-github-tracker #120

mistscale · 2022-02-28T19:49:03Z

https://lisen-github-tracker.netlify.app/

michaelchangdk

Honestly, I'm so impressed by your work. It's really elegant, concise, and clear. This minimalistic yet effective and efficient approach to both your code and your design, to me, shows a clarity of vision and problem-solving. My only 'critiques' are literally three lines in the CSS. Bravo! Five out of five stars ⭐ ⭐ ⭐ ⭐ ⭐ plus a heart ❤️ !

michaelchangdk · 2022-03-02T14:28:42Z

code/index.html

  <link rel="stylesheet" href="./style.css" />
+  <link rel="preconnect" href="https://fonts.googleapis.com">
+  <link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
+  <link href="https://fonts.googleapis.com/css2?family=Source+Code+Pro&display=swap" rel="stylesheet">
+  <script src="https://cdn.jsdelivr.net/npm/chart.js"></script>
 </head>
 <body>
-  <h1>GitHub Tracker</h1>
-  <h2>Projects:</h2>
-  <main id="projects"></main>
+  <!-- <h1>GitHub Tracker</h1> -->
+  <main id="projects" class="projects">
+  <div id="userSection" class="user-container"></div>
+  <div id="repoSection" class="repo-container"></div>
+  </main>

  <!-- This will be used to draw the chart 👇 -->
-  <canvas id="chart"></canvas>
-
+  <div class="chart-container">
+    <canvas id="chart"></canvas>
+  </div>
+  <script src="./secret.js"></script>
  <script src="./script.js"></script>
  <script src="./chart.js"></script>


Really nice HTML file! Super clean and impressed that everything was added via javascript.

The only tiny suggestion I have is adding a Favicon to your project - it helps the website look a little less "anonymous" when looking at it next to other tabs.

I like to use https://favicon.io/ to either find or create my own Favicons :) It's super easy to implement in the header,, they provide you with the code. If you do choose to use favicons in the future, remember to drop the files and images in the same folder as your index.html file.

michaelchangdk · 2022-03-02T14:33:19Z

code/style.css

+* {
+  margin: 0;
+  padding: 0;
+  box-sizing: border-box;
+  text-decoration: none;
+}


Love this - do this on all my projects as well. Makes it really easy to set all the margins and paddings from scratch. Tiny suggestion is to add these two lines if you're using border-box, it ensures that your settings work across multiple browsers:

-webkit-box-sizing: border-box;
-moz-box-sizing: border-box;

michaelchangdk · 2022-03-02T21:51:50Z

code/style.css

+.user-container {
+  display: flex;
+  flex-direction: column;
+  align-items: center;
+}


Nice use of flex here - your user-container looks really good and balanced on your site.

michaelchangdk · 2022-03-02T21:58:56Z

code/style.css

+  display: flex;
+  justify-content: center;
+  flex-direction: column;
+  align-items: stretch;


I'm not sure what the align-items: stretch; line does - I tried deleting it from your code, and it didn't produce any visible changes that I noticed. So I think this line could possibly be deleted.

michaelchangdk · 2022-03-02T22:01:20Z

code/style.css

+.repo-name {
+  background-color: #FFF;
+  padding: 1rem;
+  width: 20rem;
+}


I noticed on your mobile version that the .repo-name containers weren't centered horizontally. This can be fixed by adding the property align-self: center; to the .repo-name class. When you then expand to a larger viewport, you can change that (because if you don't, it "squishes" some of the containers so they aren't the same size) to align-self: auto; by adding a bit of code to line 55. (I'll add it below).

michaelchangdk · 2022-03-02T22:08:29Z

code/script.js

+const getRepos = () => {
+    fetch(API_REPOS_URL, options)
+    .then(res => res.json())
+    .then(data => {
+       const forkedRepos = data.filter((repo) => repo.fork && repo.name.startsWith('project'))
+       forkedRepos.forEach((repo) => {
+           repoSection.innerHTML += `
+           <div class='repo-name' id='${repo.name}'>
+            <h2>${repo.name}</h2>
+            <p>Repo link: <a href='${repo.html_url}'>here</a></p>
+            <p>Default branch: ${repo.default_branch}</p>
+            <p>Recent update: ${new Date(repo.pushed_at).toLocaleDateString('sv-SE')}</p>
+           </div>`
+       })
+       getPullRequests(forkedRepos)
+       activateChart(forkedRepos.length)
+    })


Love how you keep each function so elegant and use it to call the next functions. Your javascript is really gorgeous to read. You get precisely what you need and pass it on to the next functions. In my opinion, your javascript is a great example of perfect use of occam's razor (or the law of parsimony, the problem-solving principle that entities should not be multiplied beyond necessity).

michaelchangdk · 2022-03-02T22:13:54Z

code/script.js

+const getPullRequests = (forkedRepos) => {
+    forkedRepos.forEach(repo => {
+        fetch(`https://api.github.com/repos/technigo/${repo.name}/pulls?per_page=100`, options)
+        .then(res => res.json())
+        .then(data => {
+
+            const filterPullRequests = data.find((pull) => pull.user.login === repo.owner.login)
+
+            if (filterPullRequests) {
+                document.getElementById(`${repo.name}`).innerHTML += `
+                <p>Pull request: <a href='${filterPullRequests.html_url}'>here</a></p>`
+            } else {
+                document.getElementById(`${repo.name}`).innerHTML += `
+                <p>Pull request by team member</p>`
+            }
+            if (filterPullRequests) {
+                getCommits(filterPullRequests.commits_url, repo.name) 
+            } else {
+                document.getElementById(`${repo.name}`).innerHTML += `
+                <p>Commits by team member</p>`
+            }
+        })
+    })
+}


Perfect - honestly there is not a single thing in your javascript I can critique. It does exactly what it is supposed to and does so beautifully. Really cool way of chaining your if / else statements twice in a row, I didn't know that was possible. I will definitely be remembering that and using it in the future myself.

michaelchangdk · 2022-03-02T22:14:35Z

code/script.js

+const getCommits = (commitsURL, repoName) => {
+    fetch(commitsURL)
+    .then((res) => res.json())
+    .then (data => {
+      document.getElementById(repoName).innerHTML += `
+      <p>Commits: ${data.length}</p>`
+  })
+}}


Only tiny thing here is I noticed you forgot to add 'options' after your commitsURL in the fetch. Wow!! I can't believe I spotted something. It's miniscule. You should be really proud.

michaelchangdk · 2022-03-02T22:15:38Z

code/chart.js

@@ -1,4 +1,34 @@
+//Font-family for the chart
+Chart.defaults.font.family = 'Source Code Pro, monospace';


Love that you looked into and added the charts font - it makes the whole site very cohesive.

michaelchangdk · 2022-03-02T22:16:49Z

code/chart.js

+      const config = {
+        type: 'doughnut',
+        data: data,
+        options: {}
+      }


Doughnuts are some of my favourite charts! I used to work a lot with big data in my old job, and make dashboards for our clients, and I just think doughnuts are superior. Hehe.

Lisen Lundgren and others added 8 commits February 27, 2022 10:37

Added token and fetched repo data

a61547a

script and chart done, draft styling with flexbox

0646a02

Update README.md

617e259

Update README.md

2bbfe66

project done-ish

99e3475

project done-ish

5935f18

testing again

02575b6

Create README.md

aa3ef9e

michaelchangdk reviewed Mar 2, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

project-github-tracker #120

project-github-tracker #120

Uh oh!

mistscale commented Feb 28, 2022

Uh oh!

michaelchangdk left a comment

Uh oh!

michaelchangdk Mar 2, 2022

Uh oh!

michaelchangdk Mar 2, 2022

Uh oh!

michaelchangdk Mar 2, 2022

Uh oh!

michaelchangdk Mar 2, 2022

Uh oh!

michaelchangdk Mar 2, 2022

Uh oh!

michaelchangdk Mar 2, 2022

Uh oh!

michaelchangdk Mar 2, 2022

Uh oh!

michaelchangdk Mar 2, 2022

Uh oh!

michaelchangdk Mar 2, 2022

Uh oh!

michaelchangdk Mar 2, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -1,4 +1,34 @@
		//Font-family for the chart
		Chart.defaults.font.family = 'Source Code Pro, monospace';

project-github-tracker #120

Are you sure you want to change the base?

project-github-tracker #120

Uh oh!

Conversation

mistscale commented Feb 28, 2022

Uh oh!

michaelchangdk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants