Kevin Powell - Community•3mo ago

Why my margin does not work?

I'm working on a navigation bar and have set a margin on the .headerContainer, but it doesn't seem to be taking effect. Could you help me understand if I'm doing something wrong? Here is the codepen: https://codepen.io/hackett-lai/pen/abebjow?editors=1100

<div class="headerContainer">
  <img src="https://i.imgur.com/kF6nml7.png" />
  <div class="nav-list">
    <button class="menu-btn currentPage" active>ABC</button>
    <button class="menu-btn">DEFG</button>
    <button class="menu-btn">HIJ</button>
  </div>
</div>

body,
html {
  display: flex;
  flex-direction: column;
  align-items: center;
  margin: 0;
  padding: 0;
  width: 100%;
}

.headerContainer {
  background-color: yellow;
  width: 100%;
  padding: 20px 0px;
  display: flex;
  flex-wrap: wrap;
  flex-direction: row;
  margin: 0px 20px;
}

.headerContainer img {
  height: 54px;
  margin-right: auto;
}

.nav-list {
  display: flex;
  flex-direction: row;
  align-items: center;
}
.menu-btn {
  height: fit-content;
  padding: 4px 8px;
  font-size: 16px;
  color: var(--grey-text);
  background-color: transparent;
  border: none;
  cursor: pointer;
  transition-duration: 0.3s;
  border-radius: 4px;
  display: block;
}

.menu-btn:hover,
.currentPage {
  background-color: var(--main-green-0);
  color: var(--white);
}


:root {
  /* color theme */
  --white: hsl(0, 0%, 100%);
  --main-green-0: hsl(80, 60%, 50%);
  --grey-text: hsl(0, 0%, 38%);
}

Hackett Lai

CodePen

Ask

...

10 Replies

Chris Bolson•3mo ago

You are telling the header to be 100% wide and that is what it is doing. The margins are added either side of the element so, in this case, are outside of the viewport. In this case I would suggest that you subtract your desired margin from the width using calc `width: calc(100% - 40px);’

Sleep Twitch•3mo ago

I guess you want hij to be a bit away from the right edge? You could set a padding-right in the headercontainer

Chris Bolson•3mo ago

I would also question why you are setting the flex and width on the html and body. By default the document flow is in a column and the width is 100% so you may well create more issues by defining these.

Hayden (Noc)•3mo ago

Well my first recommendation for making a nav bar is to use a div element with a (anchor) elements for the navigation menu items for an easier experience, and remember the box model my friend, you would also need to utilize some padding <div class="mainnav" id="main nav"> <a href="abc.html">ABC</a> and so on

Sleep Twitch•3mo ago

I guess it's always best practice to only put css properties in your css that you put in intentionally and it fixes something that wasn't resulting in what you wanted. Don't put in something without knowing what it fixes.

Chris Bolson•3mo ago

At this point we don't know how the navigation is going to work so buttons may well be the correct element. It would, however, make sense semantically to place them within a <nav> element rather than a <div>

Hayden (Noc)•3mo ago

Very true

13eck•3mo ago

Fixed a bunch of stuff for you here: First, never use a button for navigation: that's the entire point of the anchor tag. Buttons are for interacting with the existing page, anchors are for navigating to a different location (within the same page or another page). Use the right tool for the job. -# you have a currentPage class so I assume it's for page navigation

<div class="headerContainer">
  <img src="https://i.imgur.com/kF6nml7.png" />
  <nav>
    <a href="/" class="currentPage" active>ABC</a>
    <a href="/">DEFG</a>
    <a href="/">HIJ</a>
  </nav>
</div>

<div class="headerContainer">
  <img src="https://i.imgur.com/kF6nml7.png" />
  <nav>
    <a href="/" class="currentPage" active>ABC</a>
    <a href="/">DEFG</a>
    <a href="/">HIJ</a>
  </nav>
</div>

/* Moved the `:root` to the top (Global variables are the very first thing, makes it easier to change them later for any reason) */
:root {
  /* color theme */
  --white: hsl(0, 0%, 100%);
  --main-green-0: hsl(80, 60%, 50%);
  --grey-text: hsl(0, 0%, 38%);
}


/* box sizing is a must! */
*,
*::before,
*::after {
  box-sizing: border-box;
}


/*
removed everything that the body and HTML element basically do already.
*/
body,
html {
  margin: 0;
}


/*
Again, removed declarations that are superfluous (`width: 100%`) or not needed for this (`flex-wrap`).
Used logical properties (`-block` and `-inline`) and only set the ones that needed setting, left the others
to be their default. Only change what you need to!
*/
.headerContainer {
  display: flex;
  flex-direction: row;
  background-color: yellow;
  padding-block: 1.5em;
  margin-inline: 1.5em;
}


.headerContainer img {
  height: 54px;
  margin-right: auto;
}

.nav-list {
  display: flex;
  flex-direction: row;
}

/*
Instead of a class only used for the nav links, I used the parent class to scope these styles.
Also used `em` instead of `px` for padding (personal choice, but will scale if/when the end user has
a different font size than the default set)
*/
.nav-list > a {
  padding: .25em .5em;
  color: var(--grey-text);
  cursor: pointer;
  transition-duration: 0.3s;
}


/*
Again, used the parent class to scope styles.
*/
.nav-list > a:hover,
.nav-list > .currentPage {
  background-color: var(--main-green-0);
  color: var(--white);
}

/* Moved the `:root` to the top (Global variables are the very first thing, makes it easier to change them later for any reason) */
:root {
  /* color theme */
  --white: hsl(0, 0%, 100%);
  --main-green-0: hsl(80, 60%, 50%);
  --grey-text: hsl(0, 0%, 38%);
}


/* box sizing is a must! */
*,
*::before,
*::after {
  box-sizing: border-box;
}


/*
removed everything that the body and HTML element basically do already.
*/
body,
html {
  margin: 0;
}


/*
Again, removed declarations that are superfluous (`width: 100%`) or not needed for this (`flex-wrap`).
Used logical properties (`-block` and `-inline`) and only set the ones that needed setting, left the others
to be their default. Only change what you need to!
*/
.headerContainer {
  display: flex;
  flex-direction: row;
  background-color: yellow;
  padding-block: 1.5em;
  margin-inline: 1.5em;
}


.headerContainer img {
  height: 54px;
  margin-right: auto;
}

.nav-list {
  display: flex;
  flex-direction: row;
}

/*
Instead of a class only used for the nav links, I used the parent class to scope these styles.
Also used `em` instead of `px` for padding (personal choice, but will scale if/when the end user has
a different font size than the default set)
*/
.nav-list > a {
  padding: .25em .5em;
  color: var(--grey-text);
  cursor: pointer;
  transition-duration: 0.3s;
}


/*
Again, used the parent class to scope styles.
*/
.nav-list > a:hover,
.nav-list > .currentPage {
  background-color: var(--main-green-0);
  color: var(--white);
}

ἔρως•3mo ago

small correction: .currentPage should be inside the .nav-list there are valid uses of that type of class name that are outside the navigation on the header (like breadcrumbs or "useful links" on the footer)

13eck•3mo ago

Good point 👍

Gaming

Programming

Why my margin does not work?