ghabebossin / prj-rev-bwfs-dasmoto Goto Github PK

[Exceeds expectations] Great work!

Great

• alt tags on images!
• Your CSS code is DRY (Don't Repeat Yourself)
• Class names are informative
• Images are in "images" directory
• Downloaded images to your local images directory and linked them locally
• Great Indentation in HTML/CSS files
• Used <div> or other tags to wrap the section of the code -- Easy to read
• Alternative font-family
• Nav bar - WOW, Amazing 👍👍👍

Suggested Improvements

• Linking images locally in Css file as well-- But great job doing that in index.html file. #1
• Multiple Fonts: #4

Overall, Perfect! 💯

Well first it looks really great! -- It matches the heading colors 👍

https://github.com/moonmooncatatat/prj-rev-bwfs-dasmoto/blob/eb6c941f2d4c626f00867d6138816d043dd6cf53/Dasmotos/Resources/css/styles.css#L1

So easiest way would be to use text-align property. The text-align CSS property describes how inline content is aligned in its parent block element.To read more:

https://developer.mozilla.org/en-US/docs/Web/CSS/text-align
https://www.w3schools.com/cssref/pr_text_text-align.asp

A block-level element always starts on a new line and takes up the full width available (stretches out to the left and right as far as it can). While an inline element does not start on a new line and only takes up as much width as necessary.

So it look like this:

From:
https://github.com/moonmooncatatat/prj-rev-bwfs-dasmoto/blob/eb6c941f2d4c626f00867d6138816d043dd6cf53/Dasmotos/Resources/css/styles.css#L3-L5

To:

ul {
  text-align: center;
  list-style-type: none; 
}

So great job in adding multiple fonts:

We use comma when we are trying to add multiple fonts:
From:
https://github.com/moonmooncatatat/prj-rev-bwfs-dasmoto/blob/eb6c941f2d4c626f00867d6138816d043dd6cf53/Dasmotos/Resources/css/styles.css#L34-L36

To:

html{
  font-family: "helvetica", sans-serif;
}

https://www.w3schools.com/cssref/pr_font_font-family.asp

Having relative paths to images.

What I do I mean?
Instead of linking to an outside source like:

https://github.com/moonmooncatatat/prj-rev-bwfs-dasmoto/blob/eb6c941f2d4c626f00867d6138816d043dd6cf53/Dasmotos/Resources/css/styles.css#L42

So it would be as similar as you have done in index.html file, so it would look like this (you have already downloaded the pattern.jpeg image locally, so great job!):

background-image: url("../images/pattern.jpeg");

Why:
It is better that we have images in your directory. Let's assume we have a website running, and you we linked those images somewhere to external URLs. So after a while they took down the images from external site, and now our website will no longer show those images.