Skip to content
OverviewSolutions
16
Comments
464
Danilo Blas
@Sdann26

All comments

  • Danilo Blas•6,300
    @Sdann26Posted over 1 year ago

    Hi Patryk!

    It has turned out quite well but there are some things to improve.

    The texts order Summary and Annual Plan have the following color assigned hsl(223, 47%, 23%)

    Also add the background color to the body background-color: hsl(225, 100%, 94%);

    You could also add animations as a small transition, for example to this rule of your CSS transition: all 300ms

    #card .cardFooter button {
    ...
    transition: all 300ms;
}

    You can apply this to any rule that has hover, active, focus, etc.

    Good coding!

    Marked as helpful
    1
  • Danilo Blas•6,300
    @Sdann26Posted almost 2 years ago

    Pedro te ha quedado bastante bien el proyecto aunque te recomendaría intentar ser más preciso con los detalles.

    Darle un min-heigth a la clase quote hace que se vea mucho espaciado entre la linea que separa la frase y el botón. Te recomendaría en todo caso quitarselo y a la clase quote darle un padding-bottom de 3.5em (Uso em ya que veo más recomendado usar medida relativas que absolutas en este caso.

    Entonces te quedaría así:

    position: relative;
text-align: center;
font-weight: 800;
width: 100%;
max-width: 640px;
border-radius: 32px;
padding: 32px 42px;
  padding-bottom: 32px;
background-color: var(--neutral-dark-grayish);
padding-bottom: 3.5em;

    Además a la paralabra ADVICE yo le pondría el atributo letter-spacing: 4px. Este atributo permite darle separación a las letras y que quede como el diseño.

    Bueno eso es lo que te recomendaría mejorar para que quede más fiel al diseño.

    Buena suerte :D.

    Marked as helpful
    1
  • Danilo Blas•6,300
    @Sdann26Posted over 2 years ago

    Hola CodingTube!

    Tu maquetación te ha quedado bastante limpia, me gusta. La funcionalidad esta bastante bien. En lo personal solo le cambiaría 4 cosas:

    a. Al <section class="rating-state"> le quitaría el margen, ya que no ayuda al centrado responsive ya que los margenes son estaticos, centra bien el horizontal pero en vertical no lo hace y por ejemplo en mi pantalla el componente se ve para marcar más abajo de lo que se debería, en cambio quitale los margenes y al body colocale además los siguientes atributos:

    { 
   display: flex;
   flex-direction: column;
   justify-content: center;
   align-items: center;
   min-height: 100vh;
}

    b. Le cambiaría el color de la letra del footer ya que no contrasta bien, no digo que deba resaltar mucho pero el negro es muy opaco más bien utilizaría un color claro por ejemplo: color: rgba(149, 158, 172, 0.5);

    c. Agregale transiciones, a los botones, esto da un toque más profesional y es bastante amigable a la vista. A los elementos con las clases :hover, :active, etc. le puedes colocolar el atributo transition: transition: all 250ms;. Puedes leer la documentación para ver que valores le puedes dar.

    d. Coloca todo el <section> dentro de un <main>. Por temas de accesibilidad siempre debes tener 1 por página que englobe todo el contenido principal. Por cierto, además de hacer eso debes colocar un h1 tambien por página, puede agregarlo dentro del <main> y colocarlo con la clase sr-only que tengas los siguiente atributos con la finalidad de que este presente en el esquema de tu página pero no se pueda ver para que no altere el diseño:

    .sr-only {
  position: absolute;
  width: 1px;
  height: 1px;
  padding: 0;
  margin: -1px;
  overflow: hidden;
  clip: rect(0,0,0,0);
  border: 0;
}

    Hay un tema más si revisas el reporte, sale que no te recomienda usar los atributos ARIA que has usado, capaz no son permitidos en el elemento que has usado, pero te recomiendo que los elimines ya que el propio reporte te lo recomienda, una ves elimines todos los errores del reporte, generá uno nuevo :D.

    Y solo eso buena suerte.

    0
  • Alejandro•90
    @AlejandroLR00Submitted over 2 years ago

    I have finished this project, it seemed simple to me. Could you give me some comment about my code.

    He terminado este proyecto, me pareció sencillo. Me podrían dar algún comentario acerca de mi código.

    QR code component

    2
    Danilo Blas•6,300
    @Sdann26Posted over 2 years ago

    Hola Alejandro!

    He revisado tu proyecto y respecto al maquetado te ha quedado 10 de 10, solo pequeños detalles con los tamaños pero te ha quedado en lo personal bastante bien.

    Respecto a tu código, si es que usas visual studio code o algún otro te recomendaría usar la extensión prettier para que formatee tu código y se vea mucho más "bonito", ya que entre etiquetas dejas vacios innecesarios además que es más legible cuando las etiqueas padres enmarcan a las hijas. Como recomendación por mi parte te diría que estudies metodología BEM para el nombramiento de tus clases, hará que todo se más ordenado y sencillo.

    Por ultimo debes eliminar los errores que te presenta el reporte de frontend mentor ya que eliminarlos te ayudará a mejorar bastante, para ellos agregan un <main> encima de <footer> y agrega todas las etiquetas dentro de <div class="card"></div> y esta dentro de <main>. Siempre debes tener un main que contenga todo el contenido principal de tu página una vez por cada página.

    Y con esto listo puedes generar un nuevo reporte y no deberían salirte ningún error.

    Eso seria todo, espero haberte ayudado :D!

    Marked as helpful
    0
  • Danilo Blas•6,300
    @Sdann26Posted over 2 years ago

    Majito, felicitaciones por acabar este reto!

    Te ha quedado bastante bien, ya mejorarlo solo son pulir pequeños detalles en el padding o hacer la sombra más traslucida ya sea usando en ves de la variable del color una variable propia para sombras con transparencia.

    Tambien has tenido en cuenta tener 0 errores así que te ha quedado genial. Ya para mejorar a futuro puedes ir estudiando:

    • Reset.css y Normalize.css, para resetear los estilos de los navegadores por defecto.
    • Metodología para el nombramiento de tus clases, podría ser BEM ya que es la más sencilla y más usada.
    • Aprender SASS que es superset de CSS el cual le da más cualidades que ya tiene por defecto.

    Solo eso por mientras si es que no lo sabes. Good Coding! :D

    Marked as helpful
    2
  • Danilo Blas•6,300
    @Sdann26Posted over 2 years ago

    Congratulations on finishing this challenge, CARREON!

    I would only recommend you to use some methodology for the naming of your classes as BEM but maybe you have seen it unnecessary as it is a small project. What I would mention is to change the <div class="attribution"> to <footer class="attribution"> since you are using it as this in your project, besides all the tags with content inside should be inside landmark tags (nav, main, footer, aside).

    That's all I could recommend you, you've got 10 out of 10 hehe.

    Good Coding!

    Marked as helpful
    0
  • Danilo Blas•6,300
    @Sdann26Posted over 2 years ago

    Guillermo, felicitaciones por acabar este reto!!

    Respecto al maquetado te ha quedado muy bien el diseño esta bastante fiel al diseño final. Has usado bastante bien las etiquetas aunque el texto Challenge by Frontend Mentor. Coded by Guillermo Mulvihill. le podrías dar un color un poco más claro para que haga contraste con el fondo azul marino.

    Respecto al div con la clase .user podrías darle border-top: 1px solid hsla(214.9, 51.6%, 70%, 0.3), para que sea un poco más transparente como el diseño final.

    Un tip para que tus diseños tengan un acabado más profesional es agregarle transiciones, osea con el atributo transition puedes generar que haya un retraso para pasar de un estado a otro generando un efecto de transición y no espontaneidad en los elementos que tienen :hover, :focus, :active, etc. Por ejemplo a la etiqueta a que tiene el nombre Jules Wyvern puedes agregarle el atributo transition: all 240ms y verás como queda con efecto bastante bueno. Por si quieres la documentación: https://www.w3schools.com/css/css3_transitions.asp

    Lo único que te recomendaría cambiar en tus etiquetas es el h2 por h1 ya que siempre vas a necesitas uno por página en tu proyecto como titulo principal y a tu proyecto el falta ya con ese cambio puedes generar un nuevo reporte de frontend mentor sin errores.

    Y bueno esa son las únicas recomendaciones que puedo darte por ahora, buena suerte :D!

    Marked as helpful
    1
  • Danilo Blas•6,300
    @Sdann26Posted over 2 years ago

    Freivys Paredes, felicitaciones por acabar este proyecto!

    Revisandolo te recomendaría darle un poco más de tamaño a la cabezera como de 200px para que no salga tan recortada.

    Por otro lado el fondo en tamaño mobile debería ser background-size: contain.

    Te recomendaría agregar transiciones a los botones y enlaces de la siguiente forma transition: all 200ms (Puedes agregarle los valores que quieras). Esto le dará un acabo más profesional al pasar el cursor, o hacer click, etc. a tu proyecto.

    Por cierto corrige los errores que te ha generado el reporte de frontend mentor, por ejemplo has usado src="" alt="" en un div los cuales no son atributos de este ya que son solo para imagenes así que lo mejor sería eliminarlos. Masomenos los errores que salen te dan una idea como corregirlos pero si no sabes me avisas por aquí. Apenas corrijas esos errores puede generar un nuevo reporte para que salga sin ningún error.

    Por lo demás te ha quedado todo bien, buen trabajo :D!

    0
  • Kristin Brooks•40
    @kristinbrooksSubmitted over 2 years ago

    My first time making something like this without a tutorial. Now I just need tons more practice. 😂

    QR code component

    3
    Danilo Blas•6,300
    @Sdann26Posted over 2 years ago

    Congratulations on finishing your first frontend mentor project Kristin!

    Whenever you finish a challenge I always recommend that you review the bug report that frontend mentor generates because it helps you fix bugs that may be more difficult to detect in the future. In your case we see that you have some accessibility errors that can be easily fixed by doing the following:

    • Change <div class="qr-component"> to <main class="qr-component">.
    • Change <div class="attribution"> to <footer class="attribution">.
    • Change <p class="text-lg"> to <h1 class="text-lg">.

    In the first two cases these are landmark tags that serve to encompass large sections of the project. You should always have a main with the main flow of the project while the footer is more for the bottom section of the projects that usually have one or another detail like links to social networks, the disclaimer, some navigation links, etc.

    By the way you should always have a single h1 at first instance you will see that the title of the page should not be the title of the card because it works as a component that will be repeated on several occasions but if you change it will serve you so that at least in this project will not give you any accessibility problem.

    And that's all I hope my comment can help you, my pleasure!

    1
  • Danilo Blas•6,300
    @Sdann26Posted over 2 years ago

    Congratulations on finishing your first challenge, sb101-beep!

    I would recommend you mainly when working with box shadown always play with the opacity for the fading effect for example box-shadow: 1px -1px 8px -3px #1f32515c; you could also make use of functions like rgba or hsla to assign color, where the a is the alpha character that adds transparency to the color.

    Finally you have to add a main tag inside the body, this is in charge of containing all the main flow of the page. You could try changing <div class="container"> to <main class="container"> to fix this.

    This will generate a new report and you should get 0 errors.

    I hope my comments are helpful :D!

    Good Coding!

    Marked as helpful
    0
  • Danilo Blas•6,300
    @Sdann26Posted over 2 years ago

    Hi Artiom Kalugin!

    Your challenge is quite good, personally I think you could improve it a bit more by giving less margin to the left and right for the title and paragraph for example:

    .h1 {
  font-size: 22px;
  color: rgb(31, 49, 79);
  margin: 24px 10px;
}

    .p{
  font-size: 15px;
  color: rgb(125, 136, 158);
  margin: 0px 10px 24px 24px;
}

    And give the title less width.

    .card {
  padding: 16px;
  border-radius: 20px;
  background-color: rgb(255, 255, 255);
  text-align: center;
  display: flex;
  flex-direction: column;
  width: 280px;
}

    And I would reduce the border-radius to 12px of the QR image.

    Finally you have two accessibility errors and both are generated by not placing the main tag inside your body.

    To do this just change <div id="app"> to <main id="app"> or place a main tag inside it. The main reason is that semantically you should always have one that encompasses all the main content of the page.

    I hope my comments are helpful :D!

    Good Coding!

    Marked as helpful
    1
  • Danilo Blas•6,300
    @Sdann26Posted over 2 years ago

    Hi iqra0001!

    Regarding the design you have done well but try to remove the comments from your HTML as it is creating conflicts in the frontend mentor report.

    Also change the <div class="main-container"> to <main class="main-container"> as it is semantically more accurate to use it in this context and in the end it has the functionality of encompassing all the main content of your project, being necessary to always put only one inside the body.

    Don't forget to generate a new report after correcting so that the challenge comes out without any error.

    If you can make the card slightly larger so that the word to goes on the second line.

    With those details it would be perfect.

    Hehehe good coding!

    Marked as helpful
    1