NW6|Bakhat Begum|HTML_CSS|Bikes-for-refugees| Week 1 #477
Conversation
✅ Deploy Preview for cyf-bfr ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
BakhatBegum
changed the title
BakhatBegum
changed the title
There was a problem hiding this comment.
A good attempt, but for next time try to pay more attention to the details! When you are a developer, that's your main job, to recreate the image they gave you exactly! You want to notice all of the small details. A good first attempt overall! You did well with the semantic HTML, adding the images and using the hover effect, but the rest of the criteria needs a little more work! :)
| Providing donated bikes and accessories to refugees and asylum | ||
| seekers in Scotland. | ||
| <figcaption class="article__content"> | ||
| <h3 class="article__title"> |
There was a problem hiding this comment.
have a look at the image provided to you. Your title looks a bit squashed to the top. How would you fix that? Maybe with some padding?
There was a problem hiding this comment.
Hello Eliza, I committed all those mistakes that you mentioned.
index.html
Outdated
| class="header__logo" | ||
| class="article__thumbnail" | ||
| src="images/spring-fundraisers_thumbnail.jpg" | ||
| alt="" |
There was a problem hiding this comment.
There is no alt tag here!
Also, have a look at your page and the page given. Does yours look the same? Try playing around with the padding of this element in the CSS!
index.html
Outdated
| <img | ||
| class="article__thumbnail" | ||
| src="images/bikes-for-refugees_logo.jpg" | ||
| alt="bikes image" |
There was a problem hiding this comment.
You don't need to say "image" in the alt tag. The alt tag is made specifically to describe images, so adding the word image is unnecessary. It's important to keep the information as concise but descriptive as possible here!
index.html
Outdated
| <img | ||
| class="article__thumbnail" | ||
| src="images/edinburgh-damascus_thumbnail.png" | ||
| alt="image for Help us cycle 4,797km" |
| background-color: #c05326; | ||
| color: white; | ||
| border-radius: 5px; | ||
| } |
There was a problem hiding this comment.
in here, add " border: none " here to remove the black outline on your buttons. Do you see how yours has one and the sample image doesnt?
styles/style.css
Outdated
| } | ||
|
|
||
| .article__title { | ||
| margin-bottom: 0.5rem; | ||
| font-size: 1rem; | ||
| font-weight: 700; | ||
| font-weight: 600; | ||
| padding-left: 20px; |
There was a problem hiding this comment.
If you have a look at the sample image, do you see how their lines align at the sides? You dont really want your title to stick out the way yours does!
Their image:
Yours:
How can you fix this? Have a look at your padding-left property here!
styles/style.css
Outdated
| @@ -108,7 +132,9 @@ p { | |||
| /* Alert */ | |||
|
|
|||
| .alert { | |||
| border-style: solid; | |||
There was a problem hiding this comment.
The sample image does not have a border here so that's not needed :)
styles/style.css
Outdated
| @@ -59,15 +64,19 @@ p { | |||
| } | |||
|
|
|||
| /* Navigation */ | |||
| nav { | |||
| padding-right: 30rem; | |||
There was a problem hiding this comment.
Do you see how your navbar skews to the left? That's because of this padding-right there! Try removing it and see if it looks more like the image
styles/style.css
Outdated
|
|
||
| .btn { | ||
| background-color: white; | ||
| color: orange; |
There was a problem hiding this comment.
For the sake of the page looking more synchronous, you always want to use similar colors. This orange doesn't match the one we used for the other button! Play around with the colors and the font-weight of this!!
| background-color: var(--grey-light); | ||
| padding-left: 30px; | ||
| } | ||
|
|
||
| .hero h1 { |
There was a problem hiding this comment.
Play around with the styling: padding and margins here. This doesnt look exactly like the picture!
|
I made changes in all the syntax that you have mentioned. |
I replaced the div's with semantic HTML tags.
I replace broken image links.
I added a style button.
I added Flexbox and hovered over the article class.