Skip to content
This repository has been archived by the owner on Oct 26, 2020. It is now read-only.

week 8 home work done #1009

Open
wants to merge 2 commits into
base: manchester3
Choose a base branch
from

Conversation

Iman-feisali
Copy link

Your Details

Your Name: Iman
Your City: Manchester
Your Slack Name: Iman-feisali

Homework Details

Module: JavaScript
Week: 8

.then(function (data) {
return data.json();
})
.then(function (newLink) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great variable name newLink - always good to see 😄

return response.text();
})
.then(function (greeting) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good variable name. All seems to be working fine to me

return data.json();
})
.then(function (newLink) {
makeImage.src = newLink.message;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure that any image you append onto the document also has an alt attribute too

makeImage.src = newLink.message;
})
.catch((error) => {
console.log(error);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe here you could also think about displaying an error message in case something goes wrong

@@ -0,0 +1,16 @@
fetch("https://xkcd.now.sh/?comic=latest")
.then(function (data) {
if (data.ok) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting that it has this ok property - I've never seen that before

})
.then(function (newData) {
const newImage = document.getElementById("image");
newImage.src = newData.img;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again don't forget the alt attribute here, Iman

<!-- Write your answer here -->
<!--
{owner} = octocat
{repo} = Hello-World

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good


2. Describe in a sentence what this API endpoint returns when all of the fields are completed?

<!-- Write your answer here -->
<!-- Provides details for a review comment. I havn't catch that much -->

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah exactly - looks to me like a pull request

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants