Skip to content
OverviewSolutions
13
Comments
12
Hassan El Jebyly
@hassaneljebyly

All comments

  • Hassan El Jebyly•490
    @hassaneljebylyPosted 7 months ago

    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
    1
  • Jeremy Helsel•670
    @JIH7Submitted about 1 year ago

    I finished this a few weeks ago and just realized I never uploaded it! My goal on this project was to be as pixel perfect to the design as possible. Any feedback is tremendously appreciated!

    Responsive Landing Page using SCSS

    1
    Hassan El Jebyly•490
    @hassaneljebylyPosted about 1 year ago

    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 :)

    0
  • Hassan El Jebyly•490
    @hassaneljebylyPosted about 1 year ago

    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
    1
  • Hassan El Jebyly•490
    @hassaneljebylyPosted about 1 year ago

    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
    1
  • Hassan El Jebyly•490
    @hassaneljebylyPosted about 1 year ago

    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
    1
  • Hassan El Jebyly•490
    @hassaneljebylyPosted about 1 year ago

    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
    0
  • Hassan El Jebyly•490
    @hassaneljebylyPosted over 1 year ago

    congrats on finishing the challenge, I have few suggestions on each card title make it positioned absolute with the cards positioned relative then do "bottom : 0;" on the titles, this will make it stick to the bottom of the card cause currently it overflows on big screens, one last thing is there can be only one h1 tag on the page: More than one H1 on a page: good or bad? good luck :)

    0
  • Nawal Malki•160
    @NawalMalkiSubmitted over 1 year ago

    Hi there 👋, I’m Nawal and this is my solution for this challenge.

    This was my first time using tailwindcss .

    Any suggestions on how I can improve and reduce unnecessary code are welcome!

    Tailwindcss easybank landing page

    2
    Hassan El Jebyly•490
    @hassaneljebylyPosted over 1 year ago

    congrats on finishing yet another challenge, I have some suggestions and I hope they help, in the "latest articles" section put "width: 100%; object-fit: cover;" on card img tags, it will fix the problem with images not filling the whole width of the card, how ever imo putting a max width on the cards then making img elements take 100% of the width plus adding "aspect-ratio: 1;" will guarantee that your images stay the same width no matter their size, also a "justify-content: space-around; flex-wrap: wrap;" on the div containing the cards will make it more responsive with much less media queries. hopefully my feedback was clear enough... cheers :)

    Marked as helpful
    0
  • Hassan El Jebyly•490
    @hassaneljebylyPosted over 1 year ago

    thank you very much for the input, the reason I validate the input before sending the request is to not waste a request on an input I already know It's not valid, there's also a requests limit on the api hence the extreme measure 😅, the map drawing on top of the "result" div is intentional the mobile design is impossible because the result div covers the map, so I made it so it changes z-index on focus, was going to make a better solution tbh but was a bit in a hurry

    0
  • Ben-Ricketts•80
    @Ben-RickettsSubmitted over 1 year ago

    really trying to get better at javascript to hopefully move onto learning some react soon. please give me any critical feedback i could use to brush up on my code. Thank you very much

    HTML, CSS &Javascript

    1
    Hassan El Jebyly•490
    @hassaneljebylyPosted over 1 year ago

    my suggestion is to create a class for unread notifications instead of specifically selecting by user name, that way your code can be easily scalable example :

    css

    .unread {
  background-color: var(--clr-unread);
 }

    js

    const unreadNotifications = document.querySelectorAll(".unread");
function changeColorWrapper() {
   unreadNotifications.forEach((notification)=>{
       notification.classList.remove("unread");
   });
}

    I suggest you check this : building a scalable css architecture

    keep building :)

    Marked as helpful
    1
  • Hassan El Jebyly•490
    @hassaneljebylyPosted over 1 year ago

    congrats looks great, however it needs more input validation, I entered "1" for the number fields and "a" for the name field and it accepted it, I suggest you use regex to identify card company.. etc, here's a useful link for that https://medium.com/hootsuite-engineering/a-comprehensive-guide-to-validating-and-formatting-credit-cards-b9fa63ec7863 good luck.

    Marked as helpful
    0
  • Kristina Garanchuk•100
    @Alaska-999Submitted almost 2 years ago

    Adaptive card form with changing state on input. It is written using react, typescript and styled-components

    Card details form

    1
    Hassan El Jebyly•490
    @hassaneljebylyPosted almost 2 years ago

    Hello 👋, Congratulations on completing the challenge! Consider a small improvement:

    input {
  cursor: text;
}
that section on the left with colorful background image{
 background-size: cover;
 background-repeat: no-repeat;
}

    also consider using regular expressions to validate form input, this project is interesting I might start it tomorrow, cheers and good luck.

    0