Week 7 project pull request from Maria Sjögren#49
Week 7 project pull request from Maria Sjögren#49molbimien wants to merge 3 commits intoTechnigo:mainfrom
Conversation
sofiawillebrand
left a comment
sofiawillebrand
left a comment
There was a problem hiding this comment.
The page follows the guidelines and is responsive. Clean layout! The links lead to the correct places. I would have prefered that the date was formed so that the day comes before month, but that is just small details!
It was difficult finding things to comment on the code, well done! The code is clean and easy to follow, the name of the variables are logical. There are some empty rows that I would remove. Overall, awesome job! :)
| Describe how you approached to problem, and what tools and techniques you used to solve it. How did you plan? What technologies did you use? If you had more time, what would be next? | ||
| The challenge this week was to adapt to the fact that the GitHub API has a rate limit of 60 requests per hour. This means that when you are fetching all the requested data in the minimum requirements, you can only reload the page a few times before you reach the API limits... | ||
|
|
||
| When I had added the minimum requirements, I therefore inserted dummmy code to the html to be able to style the page according to my likings without running into the 403 issues and the content not being displayed. I also noticed that the API request limit is applied at the network level and that I was able to work around the limitations by connecting to the mobile network on the different cell phones available in my household. |
| </header> | ||
| <main class="page-container"> | ||
|
|
||
| <section id="profile-container" class="profile-container"></section> |
There was a problem hiding this comment.
Maybe the closing tag could be placed on the row below, like you have done with the other section-elements.
| .then((res) => res.json()) | ||
| .then((data) => { | ||
| document.getElementById(`commit-${myRepoName}`).innerHTML += data.length; | ||
|
|
|
|
||
| .card a { | ||
| text-decoration: none; | ||
| color: inherit; |
There was a problem hiding this comment.
Inherit, smart! Will have to try that one out.
| a, a > span { | ||
| position: relative; | ||
| color: inherit; | ||
| text-decoration: none; | ||
| } | ||
| a:before, a:after, a > span:before, a > span:after { | ||
| content: ''; | ||
| position: absolute; | ||
| transition: transform .5s ease; | ||
| } |
There was a problem hiding this comment.
I haven't seen this before and couldn't figure out what it did, I would love it if you could show it sometime, maybe during a demo! :)
2 participants
Here's my pull request for this week's project: Build a GitHub Tracker
What?
For this project, I added code for all the requirements on the blue level. For more information about the project, see the project brief.
Find the site here:
https://molbimien-week-7-project-github-tracker.netlify.app