-
-
Notifications
You must be signed in to change notification settings - Fork 782
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
nw6 | nazanin-saedi| BikeForRefugees | [TECH ED] Bikes for Refugees | week1 #483
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for cyf-bfr ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Hello!
Congrats on your first pull request! Seems liek you've done most the work, you just need to move style.css into the styles folder and you'll be almost there :) Thanks
@@ -64,14 +64,13 @@ <h1>Bikes for Refugees</h1> | |||
Providing donated bikes and accessories to refugees and asylum | |||
seekers in Scotland. | |||
</p> | |||
<button>Donate a bike today</button> | |||
<button>Volunteer</button> | |||
<button class="donated">Donate a bike today</button> |
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.
Descriptive class names :) You need to style these a bit diffrently though to look like the image but I am sure you are aware of that. Were also need background images
<img | ||
class="article__thumbnail" | ||
src="spring-fundraisers_thumbnail.jpg" | ||
src="/images/spring-fundraisers_thumbnail.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.
Good fix
|
||
:root { | ||
--grey-dark: #292b2c; | ||
--grey-light: #e4e4e4; |
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.
Good that you're using CSS vars :)
/* hero */ | ||
|
||
.hero { | ||
background-image: url("../images/header-bike.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.
I'm afraid this link is broken, it will work in the other CSS file
/* Alert */ | ||
|
||
.alert { | ||
background-color: var(--orange-light); |
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.
All the styles here look really good
@@ -0,0 +1,295 @@ | |||
/** | |||
* |
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.
This is the wrong CSS file I'm afraid - the one in syles is the one you want this is why the styles aren't showing up. I think the work you have done is very good though, it's very close
https://github.com/CodeYourFuture/bikes-for-refugees/blob/main/styles/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.
Good work 👍 suggestions:
Navbar should be properly styled (Position/spacing/Color text/hover effects)
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.
Well done to use background-image in class hero👍 Recommendations:
Check the correct path (../images/etc); With the same class hero, the style of the buttons needs to be changed
No description provided.