Project Github tracker, Tiina Liukkonen #114
Conversation
…ed with styling the content
|
|
||
| // variable to store the filtered repos | ||
| const forkedProjects = data.filter(repo => repo.name.includes('project-')) | ||
| console.log('forked projects:', forkedProjects) |
There was a problem hiding this comment.
Here you forgot to remove console.log :)
|
|
||
| // storing the url for comments for each repo -> commented out because not used yet in code | ||
| //const commentsForRepos = repo.comments_url | ||
|
|
There was a problem hiding this comment.
Try to remember to remove the parts you're not going to use before you hand in your project
There was a problem hiding this comment.
That's a good point 👍 I tried to explain in the comments that this part is not used so the reader doesn't look for it in the features in vain. But I could have explained my reasoning a little more. These parts of code I left as comments because my plan is to use this info when I would have time to work more on this project, let's see if/when that would be possible 🙈
There was a problem hiding this comment.
That is really smart to plan on going back when you have the time, I like that!
| // Function for fetching the languages used in projects | ||
| const getLanguages = (usedLanguages, reponame) => { | ||
| fetch(usedLanguages, options) | ||
| .then(res => res.json()) | ||
| .then(data => { | ||
|
|
||
| // storing the info about used languages | ||
| const languages = Object.keys(data) | ||
|
|
||
| }) |
There was a problem hiding this comment.
I can't see on your page that you are showing the languages used for the projects? Did you change your mind and didn't want to display the different languages or is it something that doesn't work?
There was a problem hiding this comment.
That is what happened, I came across a problem with showing all the languages (one from pull request not made by me was showing the info as null in the url). So I left it there, also for the reason that I would try to solve it later as an extra feature. But this could have been explained in the comments, so good pointing that out 👍
| @media screen and (min-width:768px) { | ||
|
|
||
| .user-section { | ||
| gap: 2rem; | ||
| margin: 2.5rem 0 0; | ||
| } | ||
|
|
||
| .user-info { | ||
| gap: 1.5rem; | ||
| padding-bottom: 6rem; | ||
| } | ||
|
|
||
| .user-image { | ||
| width: 350px; | ||
| margin-bottom: 0.5rem; | ||
| } | ||
| } | ||
|
|
||
| @media screen and (min-width:1024px) { | ||
|
|
||
| .user-section { | ||
| width: 30%; | ||
| max-height: 160vh; | ||
| justify-content: space-evenly; | ||
| gap: 0; | ||
| margin: 0; | ||
| padding: 0 1rem 0 3.5rem; | ||
| } | ||
|
|
||
| .user-info { | ||
| gap: 0; | ||
| margin: 0; | ||
| padding: 0; | ||
| border: none; | ||
| text-align: center; | ||
| align-items: center; | ||
| } | ||
|
|
||
| .user-image { | ||
| width: 225px; | ||
| margin: 0; | ||
| padding-top: 1rem; | ||
| } | ||
| } | ||
|
|
||
| @media screen and (min-width:1400px) { | ||
|
|
||
| .user-section { | ||
| width: 40%; | ||
| max-height: 130vh; | ||
| padding: 1rem 0 0 3.5rem; | ||
| } | ||
|
|
||
| .user-image { | ||
| width: 275px; | ||
| padding: 0; | ||
| } | ||
| } |
There was a problem hiding this comment.
I can see that you put all the media queries for each part that needs to be responsive underneath each other, you can put all the media queries that are the same for example all with the min-width 1024px in the same curly brackets for the entire page under each other to make it even easier to find where they are and it gives you a shorter code!
There was a problem hiding this comment.
This is actually an interesting topic! I've gathered the media queries as you described in the end of the css stylesheet in previous projects. This time I tried this different approach. In this way the extra code does come from repeating the media query headline several times (the content of course stays the same), but the benefit of the positioning like this is that you don't have to scroll down the page back and forth when setting media queries or changing something for a spesific element, this was way easier when developing the code. But I'm still not totally sure myself which approach I like best in general 😅 Perhaps, gathering the media queries together in the end when finished with the styling? But a good thing that you mentioned this because it's important that the code is readable for others too 👍 🙂 Just as a curiosity, here's an interesting discussion about this topic :https://css-tricks.com/forums/topic/where-to-put-css3-media-queries/
There was a problem hiding this comment.
Also, thank you Mimmi for my first 'real' code review! 🥳 😄 🙏
There was a problem hiding this comment.
Yes, that is interesting! I'm not totally shore about with one technigo "prefer" but I do think that whatever way you do it is most important to be consistent and do the same and that you did 🥳
Mimmisen
left a comment
Mimmisen
left a comment
There was a problem hiding this comment.
Great job this week Tiina! You have done all the requirements for this project and made a beautiful page that is responsive and looks good on all devices! You have a great structure that is easy to follow. You have a lot of comments for all the parts which makes it easy to understand and you have great naming for all the functions! My feedback I have left in comments in your code!
2 participants
No description provided.