Homework week2 #5
Conversation
There was a problem hiding this comment.
I would like to see the comments fixed (most of them are still there from week 1)! For the actual task for this week, great job converting fetchJSON to promises!
homework/style.css
Outdated
| margin-left: 2px; | ||
|
|
||
| display: grid; | ||
| grid-template-columns: 33.3333% 33.3333% 33.3333%; |
There was a problem hiding this comment.
It's probably better to use flex-sizing here: grid-template-columns: 1fr 1fr 1fr; (three columns with the same flex factor = the same width)
homework/style.css
Outdated
| box-shadow: 10px 5px 5px grey; | ||
| } | ||
| span { | ||
| display: row; |
There was a problem hiding this comment.
I don't think display: row; exists. You're probably looking for display: block;, but if you're doing that you should probably use a div instead of span
There was a problem hiding this comment.
Or are you trying do make the span behave like a table, in that case it's display: table-row;
There was a problem hiding this comment.
I tried 'millions' of options looking for a way to display the contributions block in three columns having picture, name and number in separate rows. Gave it up for now since the focus is the js anyway :))
homework/index.js
Outdated
| .then((contribData) => { | ||
| contribData.forEach(contributor => { | ||
| // createAndAppend('p', contribs, { text: 'hej'}); | ||
| createAndAppend('img', contribs, { src: contributor.avatar_url, height: 40, class: 'picture' }); |
There was a problem hiding this comment.
As in week 1 I'm not a fan of this html structure as it doesn't provide any clue how to group the items. See linked comment for more details.
|
|
||
| box-shadow: 10px 5px 5px grey; | ||
| } | ||
| #forContributorsBlock { |
There was a problem hiding this comment.
I'm kind of following what you want to do here, but would prefer another solution (this one doesn't use semantic html and isn't easy to understand):
I'm assuming a html-strucuture like this:
<div class="container contributors">
<h2>Contributors</h2>
<ul class="forContributorsBlock">
<li class="contributor">
<a href="...">contributor name</a>
<img src="..." />
<span>5</span>
</li>
</ul>
</div>.contributors {
color: rgb(48, 0, 20);
margin: ...;
padding: ...;
border: ...;
}
.forContributorsBlock {
list-style: none;
}There was a problem hiding this comment.
Oh, yes that would be awesome to achieve. I think I still need to practice more DOM manipulations to be able to complete this one :(
homework/index.js
Outdated
| fetchJSON(url) | ||
| .then(repositoryData => { | ||
| const select = createAndAppend('select', root); | ||
| createAndAppend('option', select, { text: 'Click here to choose a Repository' }); |
There was a problem hiding this comment.
I'm still missing:
- Data is loaded for the first repository when the app/page loads
- The repos are sorted by name
There was a problem hiding this comment.
Sorted by name, still thinking how to start the first repo onload
There was a problem hiding this comment.
Sorting looks good! Let me know if you need any pointers for first repo onload.
There was a problem hiding this comment.
What you need to do is basically the following:
- Move the function that you send in as a callback to the eventhandler on line 53 to a separate function
- Call this function in the event-handler on line 53
- Call this function when the page loads
Is it clearer how to proceed?
No description provided.