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

All comments

  • MrLanter• 140

    @MrLanter

    Posted

    Hi, I hope you are doing well. Congratulations for this great work and the efforts that have been put into it!

    I would like to give you some suggestions for improvement:

    I strongly recommend that you use the <p> and <h1> tags for the text. This is important for readability, maintenance, and for SEO of the page on a browser. Here is some code that could improve this part:

    <h1 class="title">Improve your front-end skills by building projects</h1>
<p class="description">Scan the QR code to visit Frontend Mentor and take your coding skills to the next level</p>

    In the CSS file, from line 1 to line 13, you put a lot of selectors. There is a quick way to select all elements in the document, which should be much more convenient:

    * {
  	margin: 0;
	padding: 0;
        /* other styles */
}

    Text sizes should always be in rem and not px, because some people enlarge the text size in their browser settings and it might not respect their preferences. Here is a detailed explanation on this

    I hope these suggestions were helpful, have a nice day!

    1
  • Extendo99• 20

    @Extendo99

    Submitted

    What are you most proud of, and what would you do differently next time?

    I tried to set the dynamic width of the container in percentages, but I found that a rigid assignment of the container width would be better in this case. I would definitely start positioning the elements differently next time.

    What challenges did you encounter, and how did you overcome them?

    I had a problem with the responsiveness of the container but I used media queries.

    What specific areas of your project would you like help with?

    I definitely need help in creating responsive designs that will look good on various screens.

    Blog preview card using Flexbox

    1

    MrLanter• 140

    @MrLanter

    Posted

    Hi, congratulations for this great work!

    I am far from being a pro in responsive, but I can give you some other advice:

    Text sizes: Text sizes should always be in rem. Some people enlarge their text size in the browser settings and putting this size in px would not respect their preferences. Here is a clear explanation on this subject

    HTML indentation: It is recommended to follow a good indentation of the HTML code for better readability. I do not know if it is intentional, but it is difficult to understand the code when the child elements are not aligned.

    Semantic HTML: Using <div> repeatedly can sometimes be complicated for readability. I advise you to use semantic tags such as <main>, <article>, <footer>, ... This can also improve the page's SEO for the browser.

    I hope these tips have helped you! Have a nice day!

    0
  • MrLanter• 140

    @MrLanter

    Posted

    Hi, congratulations for this nice work and for the efforts that have been made!

    I think I can give you some suggestions for improvements:

    For centering elements, you’re using absolute positioning, which works, but I recommend trying Flexbox for a simpler solution:

    main {
  display: flex;
  justify-content: center;
  align-items: center;
}

    I noticed you’re using rem for text sizes, which is great. However, I saw a px size for <p> elements. If this was intentional, ignore this note:

    body p {
    font-size: 15px;
}

    I hope this helps! Good luck with your project and have a great day!

    Marked as helpful

    1
  • MrLanter• 140

    @MrLanter

    Posted

    Hi, I hope you’re doing well. Congrats on your project and all the hard work!

    Here are a few suggestions for improvement:

    • Semantic tags: Using more semantic HTML tags like <main>, <article>, and <footer> instead of <div> can improve both code readability and SEO.

    • Class names: Consider using more specific class names like card instead of container. It makes the code more maintainable and avoids conflicts in larger projects.

    • Font import: I noticed you import fonts from Google Fonts, but it might be more efficient to use the fonts in the project’s assets folder. This can improve performance and give you full control. Here's an example to help:

    @font-face {
  font-family: 'font-name';
  src: url('path-to-your-font') format('truetype');
  font-weight: 400;
  font-style: normal;
}

    I hope this helped you, good luck and have a nice day!

    Marked as helpful

    0
  • MrLanter• 140

    @MrLanter

    Posted

    Hi, this is a very good project and congratulations for this accomplishment!

    I think I can give you some suggestions for improvements:

    Text sizes should always be in rem. Some people enlarge the text size in the browser settings and setting this size to px might not respect their preferences. Here is a useful resource on this topic - 5 mins to read

    It is true that it is not a huge project, but I think it's better to give less vague class names than container or content for the card.

    Do you really need to wrap a <h1> and a <p> in a <div> that is itself contained in a <div>? I think it would be good to leave for example:

    <div class="text">
  <h1>Improve your front-end skills by building projects</h1>
  <p>Scan the QR code to visit Frontend Mentor and take your coding skills to the next level</p>
</div>

    without the .container.

    I hope this helps! Good luck for the future and have a nice day!

    0
  • Enziu• 40

    @DefinitelyNotPatrick

    Submitted

    What are you most proud of, and what would you do differently next time?

    I am most proud of the use of the grid along with the flex and the imagination I was able to use. Next time I won't make mistakes like I noticed now such as elements that repeat and I could have created a common class for them

    RECIPE PROJECT

    1

    MrLanter• 140

    @MrLanter

    Posted

    Hello, this project is really great, and I appreciate the effort that went into it, congratulations!

    I could just recommend you to split the main.scss file into several files and then import them all from main.scss. This method of doing would allow a better readability and maintenance of the code.

    I also noticed a part of your code quite repetitive which is a problem for maintenance, accessibility, and for the speed to write the code:

    &--one::before {
   content: '1.';
}
&--two::before {
   content: '2.';
}
&--three::before {
   content: '3.';
}
&--four::before {
   content: '4.';
}
&--five::before {
   content: '5.';
}
&--six::before {
   content: '6.';
}

    The best solution I can suggest is to leave list-style visible for both ol and ul and change the styles by doing

    li::marker {
        color: $myFavoriteColor;
        font-size: 1rem;
}

    I also noticed that you put an alt for the image, which is good but I think its content could be shorter because it can be truncated.

    You imported the text fonts from google fonts only there is already a folder that contains the fonts and I think it would be better to import them from here.

    @font-face {
    font-family: 'font-name';
    src: url('url') format('truetype'); /* format value depends on the file format */
    font-weight: 400;
    font-style: normal;
}

    I hope this really helped you, I wish you good luck, bye!

    Marked as helpful

    0
  • Mitchell Quevedo• 30

    @MitchellQuevedo

    Submitted

    What are you most proud of, and what would you do differently next time?

    I´m feeling more confident about box sizing and responsiveness. I learned that in this project can be a Good practice to set a min-height but not set a height so the content can define the height itself.

    What challenges did you encounter, and how did you overcome them?

    I thought that I should´ve used the element for the links, but that was semanticly incorrect, was the right way.

    What specific areas of your project would you like help with?

    I think I still need to improve responsiveness.

    social-links-profile

    1

    MrLanter• 140

    @MrLanter

    Posted

    Hi, this is a nice project! I think I have the solution to your responsive problem.

    You have defined a min-width for the card. Only for mobile versions that will be below the value of this property, the width will not adapt. Solution: Use a width fix: width: 350px which should adapt with modern browsers or rather width: 80% for better accessibility.

    I noticed that you are using rem for font sizes, but it seems that you might have missed applying it to the links?

    For the CSS reset I would rather advise targeting all elements with * rather than doing it in <body> because some elements like <h1>, or <p> do not adapt and remain with the browser's default values ​​for margin and padding.

    * {
  margin: 0px;
  padding: 0px;
  font-size: 1rem;
}

    Using min-height for the card can be visually good at the beginning of the project, but at the end it is better to remove it. If the user sets his text size to very small (which is very rare), there is too much space and that could bother the user. You could try by going to the browser settings to see.

    And I would like to ask you if it is intentional to have left spaces in the html or if it is just a bug for me? I would advise you not to put too many to make the code more readable.

    I hope this was useful to you, good luck

    0
  • Carina• 10

    @Carinalon

    Submitted

    What are you most proud of, and what would you do differently next time?

    Estoy orgullosa porque una vez que arme toda la base, salvo el tema de alinear el contenedor, después pude organizar el contenido, y estilizarlo sola, para mi fue un gran logro!

    What challenges did you encounter, and how did you overcome them?

    me cuesta todavía entender css, tengo que recurrir a videos para algunas configuraciones, por ejemplo el tema de los centrados, pero note que es solo cuestión de practica!

    What specific areas of your project would you like help with?

    En como saber con que etiquetas debería armar mi Html para que mi proyecto fuera mejor, y en css solo algunos aspectos de alineación de componentes o variables.

    Tarjeta con imagen QR, responsiva.

    1

    MrLanter• 140

    @MrLanter

    Posted

    Es un buen trabajo y creo que puedo dar algunas sugerencias:

    Es importante poner siempre el tamaño del texto en rem y no en px. Las personas amplían el tamaño de su texto en la configuración del navegador y la configuración de px no respeta sus preferencias. Ver aquí para una explicación detallada

    Veo que restableciste algunas propiedades predeterminadas del navegador como aquí:

    body{
  margin: 0;
  padding: 0;
}

    pero por ejemplo para p y H1 no pudieron estar de acuerdo con el margen y el relleno en 0 especificados en el cuerpo. Podrías usar * para apuntar a todos los elementos. O podrías usar el restablecimiento de CSS.

    También puede agregar un cuadro de sombra al .card como en el modelo. Es más fácil diferenciarlo con el fondo.

    Y un último detalle que quizás no te parezca tan inquietante: el desplazamiento innecesario hasta el final de la página. Es un pequeño detalle pero para una página tan pequeña preferimos no tener desplazamiento. Poner estas 2 líneas podría corregir este problema :

    .attribution {
    position: absolute;
    bottom: 20px;
}

    Espero que te haya sido útil, que tengas un buen día.

    Marked as helpful

    0
  • MrLanter• 140

    @MrLanter

    Posted

    This project is visually good and regarding the code I think I could give you some suggestions for improvements:

    Text sizes should always be in rem and not in px. This is very important because people enlarge the text in their browser settings. Putting the text size in px may not respect their preferences. If you want to display a 24 px text, to convert it to rem you have to do 24/16 = 1.5. A resource that explains it very well

    Then it would be better to separate the css in another file for better clarity. You can name it for example style.css and link it with <link rel="stylesheet" href="style.css"> from the HTML file.

    I also saw that you reset default properties in the body and that's good

        body {
      font-family: "Outfit", sans-serif;
      margin: 0;
      padding: 0;
    }

    But you can try to target all elements with * like this:

        * {
      font-family: "Outfit", sans-serif;
      margin: 0;
      padding: 0;
    }

    With this way of doing you can remove padding and margin of all elements by default on browsers. You can use more advanced CSS reset methods to avoid bugs on different browsers.

    I hope this was useful to you, have a nice day.

    Marked as helpful

    0
  • Omarhdez-218• 30

    @Omarhdez-218

    Submitted

    What are you most proud of, and what would you do differently next time?

    I am most proud of how i was able to center the qr code to the middle of the screen.

    QR code

    1

    MrLanter• 140

    @MrLanter

    Posted

    Hi! Your project is visually well done and I think you could improve on a few points:

    • the border-radius of the white box could be better connected to the image (a smaller number of px)
    • a small space between the box and the attribution text
    • the font-family on the title does not seem to match and the color either, and I think it is a problem with the import of the font-family. In the example we use a font from Google Fonts so you can watch a way to do it on youtube to find the import link. Try this line and it should work: @import url('https://fonts.googleapis.com/css2?family=Outfit:[email protected]&display=swap');

    And regarding the code, it looks pretty well structured but maybe you could add a <main></main> to wrap the main code?

    I hope these suggestions are useful to you. Your project is very cool and well designed. Have a nice day 😊

    0