Gaffari Week2 Homework #80
Conversation
There was a problem hiding this comment.
Hi @Gaffari02, your Code is working except the error handling event, almost like the previous homework as Jim's review.
I saw that you assigned your both homework in the same day so I won't ask for further changes, And because you made your contributors section.
you still have some minor issues such as some naming and some other Code Conventions, Like using the tools you have for instance, and the indentation.
But also it would be nice if we saw the adjustment we asked for in your week3 🙂.
that's it, Keep up the great job 👍.
| reject(new Error(`Network error: ${xhr.status} - ${xhr.statusText}`)); | ||
| } | ||
| }; | ||
| xhr.onerror = () => cb(new Error('Network request failed')); |
| const table = createAndAppend('table', container); | ||
| const tBody = createAndAppend('tBody', table); | ||
| const trRepository = createAndAppend('tr', tBody); | ||
| trRepository.className = "table-info"; |
There was a problem hiding this comment.
Oh I see that you have too much .className & .setAttribute methods, it's just a kind of confusing that you are using them while you have them ready in your creatAndAppend function, I highly encourage you to re-read the creatAndAppend function you have...
| trRepository.className = "table-info"; | ||
| createAndAppend('td', trRepository, {html: "Repository:"}); | ||
| const link = createAndAppend('a', trRepository,{html: repository.name}); | ||
| link.setAttribute('href', repository.html_url); |
There was a problem hiding this comment.
no need for this too you can use your tool.
if you have a tool can do a specific thing instead of you, you may use it, by using it you will have a much cleaner code plus a DRY one and easier to debug too.
| const trDescripton = createAndAppend('tr', tBody); | ||
| createAndAppend('td', trDescripton, {html: "Description:"}); | ||
| createAndAppend('td', trDescripton,{html: repository.description}); | ||
| trDescripton.className = "table-info"; |
| const trRepository = createAndAppend('tr', tBody); | ||
| trRepository.className = "table-info"; | ||
| createAndAppend('td', trRepository, {html: "Repository:"}); | ||
| const link = createAndAppend('a', trRepository,{html: repository.name}); |
There was a problem hiding this comment.
I think you should fix this as long as Jim told about it in a homework previously, this a tag may have some side effect you may not notice but HTML is just like a building on top of each others tags if you didn't follow it's rules you will end up breaking it.
looking to the documentations under the example table you will find a blue table that contains the Permitted content header aside, there you can find the valid values you can use.
| fetchJSON(repository.contributors_url) | ||
| .catch(err =>{ | ||
|
|
||
| createAndAppend('div', root, { html: err.message, class: 'alert-error' }); |
There was a problem hiding this comment.
same for the catch here, Note Jim commented on this previously.
| .then(contributors => { | ||
| contributors.forEach(contributor=> { | ||
| const ul = createAndAppend('ul', rightContainer); | ||
| ul.className = "ul"; |
There was a problem hiding this comment.
INDENTATION plus the tool you have for compose the line and improving your code better.
| li.className = "li"; | ||
| createAndAppend('img', li, {src: contributor.avatar_url, class: 'cont-img'}); | ||
| createAndAppend('a', li, {html: contributor.login, href:contributor.html_url, target:'_blank', class: 'link-name'}); | ||
| createAndAppend('div', li, { html: contributor.contributions, class: 'cont-number'}); |
There was a problem hiding this comment.
Here is a very good use of the tool you have 💯 excluding the className you have above them!
| function renderRepository(container, repository) { | ||
| container.innerHTML = ''; | ||
|
|
||
| const table = createAndAppend('table', container); |
There was a problem hiding this comment.
may repositoryDetails have a better pointing name to what this variable have or hold?
| container.innerHTML = ''; | ||
|
|
||
| const table = createAndAppend('table', container); | ||
| const tBody = createAndAppend('tBody', table); |
There was a problem hiding this comment.
if the previous comment so??
so this may be Hmmm.. detailsBody ? you may have a better one of course 😃
and smoothers you should also think about this like that.
3 participants
No description provided.