Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Log in with GitHub
OverviewSolutions33Comments18

All comments

  • Nofar Aviv• 120

    @nofaraviv

    Submitted

    I want to get better on the front-end development. Any comments or suggestions that help me improve are greatly appreciated.

    Stats preview card component

    1

    Mijail Hernandez• 950

    @MishaHernandez

    Posted

    Hello Nofar Aviv 👋

    It looks great, congratulations.

    I just have a little remark: in desktop layout the main component has a lot of margin-top and this can produce a scroll bar on some screen sizes. So I would recommend removing margin-top and using flexbox or grid to center it horizontally and vertically.

    body { display: flex; align-items: center; height: 100vh; }

@media (min-witdh: 1200px) {
   container {margin-top: 0; }
}

    I hope this is helpful to you. Happy coding 🙂

    0
  • TrakaMeitene• 505

    @TrakaMeitene

    Submitted

    Script tag is in HTML, have no idea if it is the best practice, but it's so small, I figured that it don't matter. But I con not make the button another color on mobile. It pissed me off :D This project was a little challenge for me.

    Fell free to give some feedback how I could do it better.

    Article preview with JS

    1

    Mijail Hernandez• 950

    @MishaHernandez

    Posted

    Hello TrakaMeitene 👋

    1. Adding javascript in the html is not a good practice, but if the script is very small then there is no problem and rather it turns out to be more efficient.

    2. To change the button you just need to add to the script a constant that gets the button's id and then inside the condition where the button must change color add the styles. For example:

    var x = document.getElementById ("pop");
const btnShare = document.getElementById ("btnShare")
   if (x.style.display === "none") {
     x.style.display = "block";
     btnShare.style.backgroundColor = "hsl(217, 19%, 35%)";
     btnShare.color = "white";
   } else {
     x.style.display = "none";
     btnShare.style.backgroundColor = "hsl(210, 46%, 95%)";
     btnShare.color = "";
   }

    As additional observations:

    • To initialize a variable use const or let instead of var.
    • To add styles from javascript I recommend using classList which gives you more options. [HTML DOM classList Property] (https://www.w3schools.com/jsref/prop_element_classlist.asp)

    I hope I can help you with these recommendations. Happy Coding ✌

    1
  • Mijail Hernandez• 950

    @MishaHernandez

    Posted

    Hi TrakaMeitene 👋

    Good job! Looks very similar to the challenge design. There is only small detail, you should rewrite the margin to the body (margin: 0) because this can generate a vertical scroll bar on some desktop screens.

    Greetings and happy coding! 🙂

    1
  • TrakaMeitene• 505

    @TrakaMeitene

    Submitted

    A little tricky, but at least it looks good on mobile and tablets . Made some changes, solving design problems. Thank you for comments.

    Positioning elements

    1

    Mijail Hernandez• 950

    @MishaHernandez

    Posted

    Hello TrakaMeitene. Looks great, good job. 👋

    I would recommend that in your * {...} formatting rules you always use box-sizing: border-box, so that margins and padding do not affect the size of your boxes.

    Always avoid using ID's, especially if your design doesn't use javascript. In similar elements, for example the three icons: file, folder and cloud, you could use pseudoclasses to select each one.

    Sometimes the use of clip-path can be replaced by something simpler like a correctly positioned geometric figure. For example the .sturis element could be a semi-hidden triangle in the pop-up message.

    The font family should be assigned in the body so that all elements inherit it and not have to repeat this property in several rules.

    Greetings, and continue with the challenges 👍

    1
  • Lawrence• 240

    @LawrencePryer

    Submitted

    I have been watching and following along with some HTML and CSS tutorials for the past month. This is my first effort trying to replicate a design on my own.

    I'd appreciate any feedback and suggestions for improving the way I have written my code. Hopefully it is easy to follow and that I have not included any redundant lines of code.

    Thank you in advance for your feedback!

    Huddle landing page (newbie) - my first attempt on Frontend Mentor

    2

    Mijail Hernandez• 950

    @MishaHernandez

    Posted

    Hi Lawrence,

    • The layout looks good, but I recommend using the <header> tag only as a container for the logo, the <main> tag should not have a <header> tag as the parent container, and the social media buttons could go inside a <footer tag >. I recommend checking about the "html5 semantics".
    0
  • Mijail Hernandez• 950

    @MishaHernandez

    Posted

    Hi Nizamuddin, I recommend:

    • Use the patterns as background images and avoid some complications with the positioning.
    • Check Flexbox and if possible also Grid, this will help you to improve the distribution of the elements within a container.

    Greetings, check the documentation and continue with the challenges :)

    1
  • Mijail Hernandez• 950

    @MishaHernandez

    Posted

    Hi Karim, I have some observations for you:

    • I recommend not using <br> tags to generate spacing between elements, instead use margins, padding or spacing via flexbox or grid.

    • Use <buttons> tags instead of <div> and use the font-size property instead of header tags.

    • In the action call (.start1) use a <form> tag as a container for the text box and the button. Use a padding in the main container and use margins instead of <br>.

    • In .social-media you use fontawesome icons but you must assign font css properties to give it size and color.

    Finally I recommend that you review documentation about semantic html. Greetings and continue with the challenges :)

    2
  • Mijail Hernandez• 950

    @MishaHernandez

    Posted

    Te ha quedado muy bien, sólo añadiría a la imágen de encabezado .card__head img un width: 100% para que no desborde su contenedor. Greetings and continue with the challenges.

    1
  • P
    Kalvin Hart• 140

    @kalvinhart

    Submitted

    This was my first attempt at developing something mobile first. It was also my first attempt at making a landing page.

    There are several issues that I have come accross and would appreciate any help/advice:

    1. Upon hosting the page with github pages, I seem to have lost 2 images - the patterns for the intro section that were in the ::before and ::after. They show up fine on my local machine during development, why are they not loading on the github page?

    2. With screen widths < 375px the main image scales down which then leaves the ::before pattern image lingering and looking out of place, I am not sure how to fix this. It might be hard to see what I mean seeing as the image no longer displays as per point 1 above.

    3. Again with the intro pattern images, this time the ::after image on large screens. It shows above all other elements despite my header element having a z-index of 100 and position: fixed. I am not sure why this is happening?

    Any other tips/advice would be appreciated as I'm sure I probably haven't used the mobile first approach in the most efficient way?

    Many thanks!

    Insure landing page with CSS Grid and Flexbox

    2

    Mijail Hernandez• 950

    @MishaHernandez

    Posted

    Hello, Kalvin Hart!

    • The header element is shown below the pattern because its z-index does not work for positioning with static values (position: static), so I recommend changing it to position: relative.

    Greetings and I hope my feedback has been useful to you :)

    0
  • Mijail Hernandez• 950

    @MishaHernandez

    Posted

    Hi Enol-Igareta!

    • The problem is the left margin of the image (.card__img) inside the main container ( .container), this margin has a size that overflows the grid, and finally the margins that help to center this container also overflow it from the viewport.
    • A solution would be to remove the left margin and align the image using flexbox or assigning it display: block; margin-left: auto;

    Greetings and I hope my comment has been useful :)

    1
  • Mijail Hernandez• 950

    @MishaHernandez

    Posted

    Hi Peter!

    Your solution has a good media response and is visually fine. I just have a few observations:

    • The password input should have the attribute: type = "password".
    • Buttons should have a brighter active state.
    • The tag for Terms and services should be an <a> with active status.
    • Some tags like <a> and <input> need accessibility attributes.

    Greetings and continue with the challenges 👍

    1
  • Mijail Hernandez• 950

    @MishaHernandez

    Posted

    Hi Batool H.

    Visually I see it very well. I leave you some observations:

    • The use of the picture tag is not very useful in this context, since the idea is not to group images but rather to use them as a background in body.
    • I recommend you center the .card container inside body using Flexbox.
    • You make good use of class names but it would be better if you adapt it to a methodology like BEM :)

    Greetings and continue with the challenges!

    1
  • Mijail Hernandez• 950

    @MishaHernandez

    Posted

    Hi Vitaly82!

    • In the mobile view your main container already has a margin assigned (margin: **5em** 1em;) but it also has a defined size (height: 90vh) and does not allow it to grow according to its content, it is the reason why you don't see the margin in the bottom. I recommend that you change the height of the main container to height: auto.

    • I also recommend that you make an intermediate view with 2 columns to generate a better responsive effect.

    Greetings and continue with the challenges :)

    1
  • Jose Angel Rey• 395

    @Jose-Angel-Rey

    Submitted

    Hello everyone!! This is my second Frontend Mentor challenge and I would like to receive your feedback regarding:

    • HTML structure 🏗

    • Responsive design 🖥💻📳

    • Application of BEM

    and if you have any recommendations about something that i need to improve it would be great 👍🏼

    Single price grid using HTML and CSS (Flexbox)

    1

    Mijail Hernandez• 950

    @MishaHernandez

    Posted

    Hi Jose!

    • You are using flexbox in the body to center the main container horizontally and vertically but the container has margins assigned (margin: 10% 0px 5% 0px) so a vertical scroll is generated in the desktop view. An alternative is to remove these margins and assign the body a height: 100vh. Greetings and continue with the challenges.
    3
  • Tereza• 605

    @sirriah

    Submitted

    The hardest part for me was a positioning of a "share menu". I tried to make it responsive, but I think I didn't do very well. I see, that on mobile view, the article box is smaller when the share menu is opened, but it seemed a little strange to me :D. Please leave me some feedback! 👋👩‍🦰

    Article Preview / JS, flexbox

    1

    Mijail Hernandez• 950

    @MishaHernandez

    Posted

    Hi Sirriah!

    • the .article-footer container is smaller than its child "menu-share", this is due to the margin allocated in .article-content and this finally forces you to manually calculate the size of "menu-share" in px (width: 327px; and height: 64px;) to cover the remaining space.

    • So I would recommend removing the margin from .article-content and placing in each of its children a padding that keeps the same values and can occupy more space, for example: .article-footer would have padding: 0 40px 31px 40px; and this would make it easier to assign a 100% width and height to the "menu-share".

    I hope it was understood 😅 Greetings and continue with the challenges 😃👋

    1
  • Mijail Hernandez• 950

    @MishaHernandez

    Posted

    Hi Tyler!

    1. Container margins are left over if you are already using flexbox for absolute centering (justify-content: center and align-items: center). But that absolute centering you're doing doesn't work properly because you need to assign its container (body) a height: 100vh.

    2. The blocks at the bottom (.sign-up and .benefits) are in columns but they don't cover the entire row because they still retain a width of the 50% that you assigned them, so you must rewrite that value to 100% within the media query.

    3. I encourage you to review the documentation about Flexbox and Grid, the latter provides a more comfortable way to solve this design.

    Greetings and continue with the challenges :)

    2
  • Mijail Hernandez• 950

    @MishaHernandez

    Posted

    In your css code there are some rules that have many descendant class selectors, this is not necessary if the element has its own class. Avoid adding too much specificity to your rules. Regards!

    0
  • Mijail Hernandez• 950

    @MishaHernandez

    Posted

    The shadows look very strong, but it still looks good. In css you should get used to using class selectors instead of ids and tags selectors. It would also be nice to replace float with flexbox. Saudações :)

    0