-
-
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
| <!-- 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