Skip to content
This repository was archived by the owner on Jan 14, 2024. It is now read-only.

Class9-Leila-Farsani-HTML-CSS-Coursework-Week3 - #447

Open
leilafarsani wants to merge 2 commits into
CodeYourFuture:masterfrom
leilafarsani:master
Open

Class9-Leila-Farsani-HTML-CSS-Coursework-Week3#447
leilafarsani wants to merge 2 commits into
CodeYourFuture:masterfrom
leilafarsani:master

Conversation

@leilafarsani

Copy link
Copy Markdown

Volunteers: Are you marking this coursework? You can find a guide on how to mark this coursework in HOW_TO_MARK.md in the root of this repository

Your Details

  • Your Name:
  • Your City:
  • Your Slack Name:

Homework Details

  • Module:
  • Week:

Notes

  • What did you find easy?

  • What did you find hard?

  • What do you still not understand?

  • Any other notes?

@jonnywyatt jonnywyatt left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good! You've learned a lot of techniques using grid

Comment thread index.html
/>
</div>
<div class="hamburger">
<div class="hamburger">

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The hamburger container is repeated twice

Comment thread style.css
margin-left: 10px;
}
.logo-img img {
display: grid;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

So that the image fills its grid cell and gets bigger / smaller as the viewport changes width, try removing display: grid from this element, and setting width: 100%

Comment thread style.css
}
.logo-img img {
display: grid;
opacity: 100%;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

opacity is 100% by default so this line isn't needed

Comment thread style.css
width: 250px;
height: 200px;
}
.items {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The nav isn't shown on small screens, so no need to style .items here

Comment thread style.css
font-weight: 300;
font-style: italic;
color: #0d3b19;
direction: rtl;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The direction property is for styling languages that read right-to-left, like arabic.
To right-align english, use text-align: right

Comment thread style.css
Comment on lines +62 to +63
justify-content: flex-end;
flex-wrap: wrap;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If you remove these two lines, the items will run in a row, like in the design

Comment thread index.html
<figure class="fab-img">
<img
class="main-img"
src="https://thebananadiaries.com/wp-content/uploads/2020/07/homemade-vegan-wedding-cake-recipe_5088-1333x2000.jpg"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This image is taller than it is wide (portrait), would be easier to match the design if you pick one that's wider than it is tall (landscape)

Comment thread style.css
}
.main-img {
object-fit: cover;
max-width: 50%;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The image should fill the width. Try replacing max-width with width: 100% and add a property: aspect-ratio: 3 (or similar value)

Comment thread style.css
.cakes {
grid-column: 2;
display: grid;
grid-template-columns: repeat(auto-fill, minmax(150px, 1fr));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

if you try auto-fit, the images should fill the whole width of the container, on wider screens

@jonnywyatt jonnywyatt added the reviewed A mentor has reviewed this PR label Nov 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

reviewed A mentor has reviewed this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants