-
-
Notifications
You must be signed in to change notification settings - Fork 43
Daniel carter week 2 website #81
base: master
Are you sure you want to change the base?
Daniel carter week 2 website #81
Conversation
@Codeama hopefully this one will work |
Hi @carterd888, I see what the issue with your previous pull request was. It's just a small problem with the file paths to your CSS and image files. The error happens because I opened the HTML file for your website directly into my browser where the URL is Here's a screenshot of my console when I first loaded the site: If you're unsure of anything around absolute/relative links, here's a good primer from MDN: https://developer.mozilla.org/en-US/docs/Learn/HTML/Introduction_to_HTML/Creating_hyperlinks On the website, overall I think you've done a good job with a few small issues: Pros
Cons
i.e. it should be structured like:
instead of
This allows you contain and style the icon and text together. There are a few small inconsistencies with the design spec but they are very small things like the logo being a slightly different size. Like I said though it's a good job overall - hope that feedback is helpful! 👍 |
It's indeed a relative path issue @carterd888. Thanks for spotting that @EmilePW. So try and update your CSS |
week2/2-website/css/style.css
Outdated
} | ||
|
||
#introducingKarma { | ||
background-image: url("/2-website/img/first-background.jpg"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update the path to "../img/first-background.jpg"
week2/2-website/index.html
Outdated
@@ -8,14 +9,62 @@ | |||
<link href="//maxcdn.bootstrapcdn.com/font-awesome/4.2.0/css/font-awesome.min.css" rel="stylesheet"> | |||
<link rel="stylesheet" href="css/normalize.css"> | |||
<!-- Add a link to your CSS file here (use the line above to guide you) --> | |||
<link rel="stylesheet" href="css/style.css"> | |||
<link rel="shortcut icon" type="image/x-icon" href="favicon.ico"> | |||
<link rel="stylesheet" href="/2-website/css/style.css"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update this to: "./css/style.css"
week2/2-website/index.html
Outdated
<img id="device" src="img/icon-devices.svg" alt="device logo"> | ||
<img id="coffee" src="img/icon-coffee.svg" alt="coffee logo"> | ||
<img id="refill" src="img/icon-refill.svg" alt="device logo"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And these too as suggested above.
week2/2-website/index.html
Outdated
<header> | ||
<nav> | ||
<div class="navigation-container"> | ||
<img id="karma-logo" src="img/karma-logo.svg" alt="karma logo"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above.
It's all good now, Daniel. :D |
made a test comment