Skip to content
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 | Fikret-Ellek | HTML-CSS | Bikes-For-Refugees | week 1 #478

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

fikretellek
Copy link

No description provided.

@netlify
Copy link

netlify bot commented Sep 23, 2023

Deploy Preview for cyf-bfr ready!

Name Link
🔨 Latest commit fe162a2
🔍 Latest deploy log https://app.netlify.com/sites/cyf-bfr/deploys/650f0d49bf4375000877b887
😎 Deploy Preview https://deploy-preview-478--cyf-bfr.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

@musayuksel musayuksel left a comment

Choose a reason for hiding this comment

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

Overall, great job Fikret! Keep going!

Comment on lines +114 to +129
.orange {
background-color: #e95b1e;
color: white;
margin-left: 25px;
border-radius: 4px;
padding: 6px 6px;
border: 0px;
}

.white {
color: #c05326;
background-color: white;
border-radius: 4px;
padding: 6px 6px;
border: 0px;
}

Choose a reason for hiding this comment

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

You have successfully applied the desired styles to your buttons. However, I wanted to bring up an important concept in programming called DRY code, which stands for "Don't Repeat Yourself."
In your code, I see that you have repeated the same set of styles for different button classes, such as "orange" and "white."
Next time, you can consider creating a single CSS class that contains the common styles like;
button { //common styles
border-radius: 4px;
padding: 6px 6px;
border: 0px;
}
.orange {
background-color: #e95b1e;
color: white;
margin-left: 25px;
}
//same idea for the white one


button:hover {
background-color: rgb(55, 178, 235);

Choose a reason for hiding this comment

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

You have used both RGB and hexadecimal color codes. To improve consistency, you can choose one format. Both RGB and hexadecimal color codes are valid and widely supported, so it's up to your personal preference.

<img
class="article__thumbnail"
src="images/spring-fundraisers_thumbnail.jpg"
alt="pink sakura tree branch"

Choose a reason for hiding this comment

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

Great catch on adding the alt text for the image! Including alt text is a great accessibility practice. Well done.

@fikretellek
Copy link
Author

Thank you very much @musayuksel

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

Successfully merging this pull request may close these issues.

2 participants