Skip to content
OverviewSolutions
16
Comments
37

enochlee

@iamenochlee600 points

Building the web...

I’m currently learning...

React JS.

Latest solutions

View more solutions

Latest comments

  • enochlee•600
    @iamenochleePosted over 2 years ago

    This challenge does not really define any hierarchy for semantic text, so you do not need to worry about what heading tag you should use, congrats on your solution, keep coding!

    1
  • enochlee•600
    @iamenochleePosted over 2 years ago

    1)add to the body { width: 100vw; overflow-x: hidden} 2)There are several ways to toggle the navbar with knowledge of javascript. You can go all in using javascript just like this, make sure the navbar has a display of none in the css limited to mobile media query,

    //grabing the elements, 
const toggler = document.querySelector(".menu-toggler");
const navBar = document.querySelector(".list--container");
const body = document.body;
const icon = toggler.querySelector("img");

//opening navbar and closing navbar

/*creating custom function to handle the click*/
const handleOpen = (el) => {
  el.style.display = "block";
  icon.src = "./assets/images/icon-menu-close.svg";
  icon.alt = "Close Navigation Menu";
  body.style.overflow = "hidden";
};

const handleClose = (el) => {
  el.style.display = "none";
  icon.alt = "Open Navigation menu";
  body.style.overflowY= "scroll";
  icon.src = "./assets/images/icon-menu.svg";
};



toggler.addEventListener("click", () => {
  if (navBar.style.display === "block") {
    handleClose(navBar);
  } else if (navBar.style.display ==="none"){
    handleOpen(navBar);
  }
});

/*closing the navbar on click outside*/
navBar.tabIndex = 0
document.addEventListener("click", () => {
  if (
    navBar !== document.activeElement &&
    body.getBoundingClientRect().width < 720
  ) {
    if (document.activeElement !== toggler) {
      handleClose(navBar);
    }
  }
});

    if you prefer a css approach you will have to add and remove a class of "open" to it when the button is clicked, the use that class to handle the display in the css.

    Keep Coding,

    EDIT: cleaned up the function

    EDIT2: While that works it has a bug, you have to click twice to open the navbar at first , here is a new solution

    
//custom functions
function handleClose(el) {
  el.classList.remove("open");
}

function handleIcon() {
  if (toggler.classList.contains("open")) {
    icon.src = "./public/assets/images/icon-menu-close.svg";
    icon.alt = "Close Navigation Menu";
  } else if (!toggler.classList.contains("open")) {
    icon.src = "./public/assets/images/icon-menu.svg";
    icon.alt = "Open Navigation Menu";
  }
}

//toggling navBar
toggler.addEventListener("click", () => {
  toggler.classList.toggle("open");
  navEl.classList.toggle("open");
  handleIcon();
});

//closing on click outside
navEl.tabIndex = 0;
document.body.addEventListener("click", () => {
  if (document.activeElement !== toggler) {
    if (
      navEl !== document.activeElement &&
      document.body.getBoundingClientRect().width < 720
    ) {
      toggler.classList.remove("open");
      handleClose(navEl);
      handleIcon();
    }
  }
});

css

.open{
display: block}
    Marked as helpful
    0
  • enochlee•600
    @iamenochleePosted over 2 years ago

    Your media-query kicks in late, leaving your solution to stretch all the way to 1500px, reduce this to 900px, also add this style.

    @media screen (min-width: 500px) and (max-width: 900px) {
.container {
padding: 10em;
}
button {
margin-bottom: 1em;
}

    I see you made use of semantic elements and relative unit, good job, you should also start approaching layout from a mobile-first approach. Keep Coding

    Marked as helpful
    1
  • enochlee•600
    @iamenochleePosted over 2 years ago

    Hey there, congratulations on your Challenge, Your solution seems to have a vertical scrollbars, well you can correct this, by using this styles

    body{
margin: 0;
width: 100%;
overflow-x: hidden;
}

header{
width: 75%;
}

.pattern{
left: 69em
}

    But this is not an efficient way, a much better way is to add the pattern as a background-image to the body for just tablet to desktop screen-sizes, since its not needed on mobile, or perhaps reposition outside the viewport for mobile, then position it top, right, i will link an article on working with two or more background image, also an article on css resets. Keep coding!.

    background-images css resets

    0
  • enochlee•600
    @iamenochleePosted over 2 years ago

    I see you are using grids, your layout breaks between 550px and 900px, ideally a tablet screen, first i think you should redefined the grid-template-column at this particular sizes to repeat(2, 1fr), since you are using tailwind that should be yourmediaquery:grid-col-2`, you can then reste it back to three at large screen sizes, while from 900px to 1200px, increase the width of the container, so the grid has more space. You can create custom media query in tailwind, check out the docs. Do well to check your website on responsive mode in your browser. i hope this helps, Happy coding, Great work!.

    Marked as helpful
    1
  • enochlee•600
    @iamenochleePosted over 2 years ago

    Looks good!, i noticed some few stuff, first i think you should disable the button or give a notification while the link is being shortened, makes for awareness, closing the mobile nav on click outside, also at 1600px screen width your block-padding for the page is very little, you might want to keep the same padding it had at 1440px. Happy Coding!

    Marked as helpful
    0
View more comments