Skip to content
OverviewSolutions
13
Comments
12

Hassan El Jebyly

@hassaneljebylyMorocco490 points

I’m a mysterious individual who has yet to fill out my bio. One thing’s for certain: I love writing front-end code!

Latest solutions

View more solutions

Latest comments

  • Hassan El Jebyly•490
    @hassaneljebyly

    congrats on finishing challenge, It looks good, but search and filter by region is not working. you can use the code below to make searching and filtering work, using the states from select and search input as conditions for filtering, then map over the filtered list and display it.

    const filteredCountryList = countries.filter((country) => {
    if (
       // filter by region
      (selectInput === "" || selectInput === country.region.toLowerCase()) 
       &&
       // filter by name
      country.name.common.toLowerCase().includes(searchInput.toLowerCase())
    )
      return country;
  });
    Marked as helpful
  • Hassan El Jebyly•490
    @hassaneljebyly

    looks great, not perfect but great, my advice to you is try and use css custom properties more, even when using scss, you don't want hardcoded values in your main css file, I use scss loops and variables to build :root with custom properties and utility classes, that way your main css isn't dependent on sass to be scalable and easy to maintain, one other is it's better to use em or rem for margins and paddings in order that typographic integrity is maintained when text is resized by the user, px value are too absolute, also try naming things and avoid nesting selectors on complex pages, that's all I have, if anyone else reading this please correct me if I'm wrong :)

  • Hassan El Jebyly•490
    @hassaneljebyly

    hey and well done for finishing the project, to answer your question in order to provide assistive technology users with a logical view of the page structure all sectioning elements <main> <nav> <header>... must have a defined role="" attribute, all sectioning elements automatically define one with the exception of the <section> tag, it only defines region when it has an accessible name using aria-labelledby or aria-label or title. the perfect fix to this in my opinion is to add an <h2 id="last-news-section-title">(title of your choice to describe the section)</h2> inside that section the add aria-labelledby="last-news-section-title" attribute to the section, It will be displayed on the page but you can hide it by adding visually-hidden class to the h2, It won't be displayed but assistive technologies will see it.

    css

    .visually-hidden {
  clip: rect(0 0 0 0);
  clip-path: inset(50%);
  height: 1px;
  overflow: hidden;
  position: absolute;
  white-space: nowrap;
  width: 1px;
}

    I hope this helps :)

    source

    Marked as helpful
  • Hassan El Jebyly•490
    @hassaneljebyly

    congrats on finishing this project looks solid and responsive, one thing I noticed is when you click to close the nav then resize to desktop It stays closed, that's because your javascript changes the style attribute, instead what I would suggest is writing a class to do the opening and closing of the nav via nav.classList.toggle("open")

    nav {
 transform: translateX(0%);
}

.open {
 transform: translateX(200%);
}

    this way transform is removed from the element. another thing is when you resize the window you can see the nav slide to right when it hits the breakpoint, that's because you have transition: 0.6s; changing it to transition: transform 0.6s ease; solves the issue, or you can just stop animations during window resizing.

    this might be useful : Responsive navbar tutorial using HTML CSS & JS

    good luck :)

    Marked as helpful
  • Hassan El Jebyly•490
    @hassaneljebyly

    Congrats on finishing this, looks neat and solid, just one suggestion put display: grid; place-items: center; on the <button> tag with the star icon (why not a div tho ? a button tag that's not used as a button will just confuse screen readers), plus you may want to add cursor: pointer on rating buttons, other than that it all looks great.

    Marked as helpful
  • Hassan El Jebyly•490
    @hassaneljebyly

    congrats, looks like It's just you and me who made the thing flip lol, I like your approach with the animation, just one observation the timer segments don't seem to be responsive, maybe try a clamp on its width, and make everything inside a 100% in width that's how I made it responsive.

    Marked as helpful
View more comments