Skip to content
OverviewSolutions
6
Comments
12

hebrerillo

@hebrerillo350 points

I love front end development and I am here to sharpen my skills by learning from all of you.

I’m currently learning...

Vanilla JavaScript, HTML5, CSS3, responsive implementations.

Latest solutions

Latest comments

  • hebrerillo•350
    @hebrerilloPosted over 2 years ago

    Hello Catherine!

    For a portrait screen size mobile like 710 x 375, when you open the navigation menu, the links are not displayed properly. To fix that, just decrease the 'margin-top' in this rule:

    .nav.active .menu { gap: 1rem; margin-top: 50%; }

    A better approach would be to statically position both the '.amburger' and the '.menu' elements.

    Marked as helpful
    0
  • hebrerillo•350
    @hebrerilloPosted over 2 years ago

    I was waiting eagerly for someone who completed this challenge using vanilla JavaScript.

    The code looks very concise, readable and maintainable, so congratulations on this challenge. I also think you probably studied a JavaScript course by Jonas. I also studied the same course and it is fantastic.

    I have only one thing. In the 'helpers.js' file, you add an extra promise to include a timeout to perform the request. That is a good solution.

    However, there is no need to add an extra promise, because the 'fetch' call have an 'options' argument when you can include an 'abortion' object. Here is an example: https://stackoverflow.com/questions/46946380/fetch-api-request-timeout

    I also implemented that technique in my challenge. It works like a charm!

    1
  • hebrerillo•350
    @hebrerilloPosted over 2 years ago

    Good job Omar! Just one thing. If you display your site on a smartphone in the landscape, and open the right popup, you will realize that the bottom buttons are not visible. To fix it, just set a scroll to the 'aside' element.

    aside { overflow: scroll; }

    Marked as helpful
    1
  • hebrerillo•350
    @hebrerilloPosted over 2 years ago

    Very good job Tochi! Just one thing: If you display the site on a smartphone and on the landscape, you will realize that the site goes up too much and is not visible. Try adding this media query:

    @media screen and (max-height: 560px) { .card { top: 0; transform: translate(-50%, 0%); } }

    Because your '.card' component is 567px height, when the view port is close to that height, set the top of the card to the top the view port.

    0
  • hebrerillo•350
    @hebrerilloPosted over 2 years ago

    I forgot to say that I also welcome general feedback, thanks!

    0
  • hebrerillo•350
    @hebrerilloPosted over 2 years ago

    Good job man! I just saw one problem. If you use a view port width of 450px, you will realize that the logo and the links in the header are stick together.

    One way to solve this problem is to set this style in the header for mobile screens: flex-direction: column;

    And then set back 'flex-direction:row' in the media query for desktops (min-width:768px)

    The rest of the implementation is very good!

    Marked as helpful
    1
View more comments