Conversation
| Replace this readme with your own information about your project. | ||
|
|
||
| Start by briefly describing the assignment in a sentence or two. Keep it short and to the point. | ||
| This week the project was to fetch information from Github about my repositories and pull requests and display the information on a page. We were also supposed to compare data in a chart. |
There was a problem hiding this comment.
Nice and to the point project brief!
| }; | ||
|
|
||
| const config = { | ||
| type: 'bar', |
There was a problem hiding this comment.
I really like that you used a bar chart to showcase your projects and your progression. Also that you put it on top of the page! The chart is also responsive which is great :)
| // Fetching profile info | ||
| const addingProfile = () => { | ||
| fetch(`https://api.github.com/users/${username}`) | ||
| .then((res) => res.json()) | ||
| .then((profileInfo) => { | ||
| profile.innerHTML += ` | ||
| <img src="${profileInfo.avatar_url}"> | ||
| <a class="userlink" href="${profileInfo.html_url}" target="_blank">${profileInfo.login}</a> | ||
| <p>${profileInfo.bio}</p> | ||
| ` | ||
| }) | ||
| } |
There was a problem hiding this comment.
I really like how you wrote your code here. Clean, good naming and nice that you made a link to your profile on github! :)
| let updated = new Date(repo.updated_at).toLocaleDateString() //Turning date and time into date | ||
| let upperCase = `${repo.name[0].toUpperCase()}${repo.name.slice(1)}` //Making first letter of string to uppercase |
There was a problem hiding this comment.
Great with the comments and nice to know how you wrote the code to make the first letter uppercase (I'm taking that with me since I tried that too but I didn't get it to work) :)
| <ul> | ||
| <li><span class="title">Most recent update:</span> ${updated}</li> | ||
| <li><span class="title">Default branch:</span> ${repo.default_branch}</li> | ||
| <li><a class="repo-link" href=${repo.html_url} target="_blank">Link to repository</a></li> | ||
| <li id=commit-${repo.name}><span class="title">Number of commits:</span> </li> | ||
| </ul> |
There was a problem hiding this comment.
Smart that you used an unordered list here!
| findingPulls(forkedRepos) // Bringing all filtered repos to the next function | ||
| callingChart (forkedRepos.length) //Bringing all filtered repos to the chart |
There was a problem hiding this comment.
Once again, good naming and I can really see that you made these comments for you to be able to understand what you're doing and that is so great, so helpful.
code/script.js
Outdated
|
|
||
| const findingPulls = (repos) => { | ||
| repos.forEach((repo) => { //For all filtered repos I fetch each pull request | ||
| fetch(`https://api.github.com/repos/Technigo/${repo.name}/pulls`) //, options |
There was a problem hiding this comment.
The comment here, is this for the use of a token?
There was a problem hiding this comment.
Yes, I must have missed it when I removed my token. Will remove it now, thank you!
| const pulls = data.find((pull)=> pull.user.login === repo.owner.login) //Comparing all pull requests from Technigo to show only the ones with me as owner | ||
| if (pulls !== undefined ) { // If pull requests exist = | ||
| findingCommits(pulls.commits_url, repo.name) //passing the commits of these pull requests to the next function | ||
| } else { // If pull requests does not exist = display this | ||
| document.getElementById(`commit-${repo.name}`).innerHTML += ' Group project/no pull request' | ||
| } |
There was a problem hiding this comment.
Nice way of writing this if/else statement.
code/style.css
Outdated
| background: #FFECE9; | ||
| box-sizing: border-box; | ||
| background: #83c5be; | ||
| /*006d77*/ |
There was a problem hiding this comment.
Maybe remove this line of code since it's not being used.
| } | ||
|
|
||
| ul { | ||
| list-style-image: url(./images/GitHub-Mark/PNG/GitHub-Mark-32px.png); |
| padding: 1vh 2vw; | ||
| } | ||
|
|
||
| @media (min-width: 668px) and (max-width: 1024px) { |
There was a problem hiding this comment.
Page looks nice in responsive mode and you have built your page doing mobile first which is great. The only thing I caught was that in mobile view the data in the project "boxes" seem to be centralized for the ones that showcase the number of commits, but for the group projects (0 pulls) the text will not be centered. I tried to play around in the inspector tool but I was unable to find a solution for this... :( Other than that, really good job with this page, Ida! :)
There was a problem hiding this comment.
I tried fixing it too but I believe it is centralized on all of them, just that the content is wider on the ones without pull requests, making it look odd. Didn't know if I should keep it centered or not...
code/style.css
Outdated
|
|
||
| .repos { | ||
| width: 20vw; | ||
| min-width: 210px;; |
There was a problem hiding this comment.
A tiny detail but here's two little semi colons together.
CamillaHallberg
left a comment
CamillaHallberg
left a comment
There was a problem hiding this comment.
You've done a really great job in creating your own GitHub tracker! I know that this week was very challenging for us so I want you to know that I think your code looks really good, I understand what you have done and I truly appreciate that you have made relevant comments. I have only made some few smaller comments that you can check if you get the time. Your code follows the guidelines and checks off on the blue requirement level. Be proud of this project. You've done such a good job! 🌟
2 participants
No description provided.