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

London 9 - Natalie Zablotska- HTML & CSS - Week 3 - #451

Open
nataliiazab wants to merge 17 commits into
CodeYourFuture:masterfrom
nataliiazab:master
Open

London 9 - Natalie Zablotska- HTML & CSS - Week 3#451
nataliiazab wants to merge 17 commits into
CodeYourFuture:masterfrom
nataliiazab:master

Conversation

@nataliiazab

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

@nataliiazab

nataliiazab commented Nov 11, 2022

Copy link
Copy Markdown
Author

@Hamisakim

Hi Sami,
Could you please advice me on how to fix the following issues?

  1. To fix the overlapping of the navbar when changing the screen size to smaller, but not mobile.
  2. To make the size of all 4 pictures below equal squares (currently they are rectangular).

Thanks!!!

@nataliiazab

Copy link
Copy Markdown
Author

@SanaAsaf

@Hamisakim Hamisakim 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.

Great Work Natalia!
Look into restructuring the navbar. We want our code to be as readable as possible. Let me know if this isn't clear enough :)

  1. Use gap to the flex parent instead of adding a margin to the children.
  2. use fixed values for the images. Experiment using min-width and max-width etc.
    width: 20rem;
    height: 20rem;

Comment thread style.css Outdated
}

.nav-item {
margin-left: 5rem;

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 will always give the items a margin-left, even on smaller screens, causing them to overlap

Comment thread style.css

.navbar {
display: flex;
justify-content: flex-end;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

change to justify-content: center;

Comment thread style.css
display: flex;
justify-content: space-between;
align-items: center;
flex-direction: row;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

we can add gap:2%; here

Comment thread index.html
</section>
<div class="body">
<header class="header">
<nav class="navbar">

@Hamisakim Hamisakim Nov 14, 2022

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider how you've built the navbar here -- it's a lot of nesting, making it harder to debug and style.
something like the below

            <nav class="navbar">
<div class=nav-items>
                <a href="proxy.php?url=https%3A%2F%2Fgithub.com%2FCodeYourFuture%2FHTML-CSS-Coursework-Week3%2Fpull%2F451%23" class="nav-link">Home</a>
                <a href="proxy.php?url=https%3A%2F%2Fgithub.com%2FCodeYourFuture%2FHTML-CSS-Coursework-Week3%2Fpull%2F451%23" class="nav-link">Cakes</a>
                <a href="proxy.php?url=https%3A%2F%2Fgithub.com%2FCodeYourFuture%2FHTML-CSS-Coursework-Week3%2Fpull%2F451%23" class="nav-link">Orderin</a>
                <a href="proxy.php?url=https%3A%2F%2Fgithub.com%2FCodeYourFuture%2FHTML-CSS-Coursework-Week3%2Fpull%2F451%23" class="nav-link">Lessons</a>
                <a href="proxy.php?url=https%3A%2F%2Fgithub.com%2FCodeYourFuture%2FHTML-CSS-Coursework-Week3%2Fpull%2F451%23" class="nav-link">About</a>
</div>
                <div class="hamburger">
                    <span class="bar"></span>
                    <span class="bar"></span>
                    <span class="bar"></span>
                </div>
            </nav>

@Hamisakim Hamisakim added the reviewed A mentor has reviewed this PR label Nov 14, 2022
@nataliiazab

Copy link
Copy Markdown
Author

@Hamisakim thank you very much for all the comments and for reviewing my code! Everything is clear!

@nataliiazab nataliiazab changed the title London 9 - Natalie Zablotska - HTML & CSS - Week 3 London 9 - Natalie Z.- HTML & CSS - Week 3 Apr 18, 2023
@nataliiazab nataliiazab changed the title London 9 - Natalie Z.- HTML & CSS - Week 3 London 9 - Natalie Zablotska- HTML & CSS - Week 3 Apr 24, 2023
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