Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Oops! You need to be logged in to do that
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

Community Feedback

  • Solution screenshot
    • HTML
    • CSS

    Html CSS Flex box

    1
    Danilo Blas4,440 | Posted 31 minutes agocommented on Gustavo's "NFT preview card component" solution

    Hi Gustavo!

    Just a recommendation since in general I think your project is well done.

    Add to the name of the NFT and the creator the property transition: color 400ms (the first value is the property to apply the transition and the second is the delay) this to generate a color change with a slight transition so that the change is not so abrupt, this will allow you to make nice animation later.

    I would also recommend you to learn BEM methodology, which is a methodology for naming your classes, that will help you to have it more organized, the same level of specificity in your style sheets and helps teamwork when handling a methodology like this. Also you could learn about Reset CSS and Normalize CSS which are stylesheets to reset the style for all browsers and not have to use the * in your stylesheet to reset the values.

    I hope my comments are helpful.

    Good Coding!

    0
  • Solution screenshot
    • HTML
    • CSS

    NFT preview card component

    1
    P
    Vanza Setia13,025 | Posted about 2 hours agocommented on Finney's "NFT preview card component" solution

    Hi, Finney! 👋

    Some recommendations from me.

    • Alternative text should not be hyphenated (like code).
    • I suggest leaving the alt="" empty for decorative images. In this case, all images except the avatar image are decorative images.
    • Remove the word picture any words that have the same meaning. It's one of the common mistakes that should be avoided when writing alternative text.
    • There should not be text in span and div alone whenever possible. Instead, wrap the text with a meaningful element like a paragraph element.
    • Wrap all elements that have interactivity with interactive elements like anchor tag.
    • Code your font size in rem, not pixels. This means the text size will be responsive if people want to adjust the size.

    Hope you find this useful!

    0
  • Solution screenshot
    • HTML
    • CSS

    Responsive landing page with curved sections

    1
    P
    Vanza Setia13,025 | Posted about 2 hours agocommented on Andrew Ezeani's "Huddle landing page with curved sections" solution

    Hi! 👋

    I notice that the font family is not the same as the design. So, my first recommendation is to use the correct font family.

    Here are several suggestions from me.

    • I suggest wrapping the logo img element with an anchor tag. It's a common thing that the logo will navigate the user to the home page. For example, clicking the Frontend Mentor logo will navigate you to the /home page. But, if you are not logged in, it will navigate you to the root page (https://www.frontendmentor.io/).
    • I would expect the "Try It Free" button as a link element. As a user, I think the button is going to navigate me to another webpage. The same goes for the other buttons.
    • header should not contain the page content. So, I suggest moving the page content inside the main element. As a result, the only things that live inside the header are the logo and the "Try It Free" button.
    • I suggest leaving the alt="" empty for all decorative images, like the illustrations, background images, and icons. A page full of decorative images with excessive alt-text adds noise to the page.
    • Always specify the type of the button if you ever need to use it.
    • Add aria-label to the social media links. Screen reader users won't be able to know those social media links if there's no text content or label.
    • Remove all the styling from the html element. Setting max-width: 100vw and font-size: 16px is not necessary because by default the page is 100% width and has 16px. Also, it's not recommended to change the html or :root font size. By doing that, you won't allow the user to change the font-size based on their needs.

    For your information, the sizes on the style-guide.md have nothing to do with the media queries. They are telling you that "this is how your website should look like at these screen sizes". As frontend developers, we should keep making your website looks good in between those screen sizes.

    Currently, on mobile landscape view, the site is not looking well.

    That's it! Hope this helps.

    0
  • Solution screenshot
    • HTML
    • CSS

    QR code component HTML/CSS

    3
    Danilo Blas4,440 | Posted about 2 hours agocommented on Kathy Sandoval's "QR code component" solution

    Hi Kathy!

    I would recommend changing the text size.

    .text p {
      color: #9699a0;
      padding: 0 8px;
      font-size: 16px;
    }
    

    I know that in style-guide.md says 15px but for your design I think that if you put 16px it looks the same.

    Leaving aside the design I recommend you always examine the error reports that frontend mentor generates. In this case <div class="rectangle"> you should change it to <main class="rectangle"> since you always need to have only one main per project inside the body because it contains all the central content of the page.

    Following with the report you should always add the alt="" attribute in the images like <img src="images/image-qr-code.png" alt="QR Code"> because it helps people who use screen readers ie blind people for example because the screen reader is in charge of interpreting these images.

    You should also change <h2>Improve your front-end skills by building projects</h2> to <h1>Improve your front-end skills by building projects</h1> because like the main you should always have one per page since it is the title of the page and the other h2, h3,.... function as subtitles one of the other and so as if it were a report.

    I hope my comments are helpful for you, all these accessibility issues must always be taken into account.

    Good luck :D!

    1
  • Solution screenshot
    • HTML
    • CSS

    QR Code Component With React and Tailwind CSS

    4
    Danilo Blas4,440 | Posted about 2 hours agocommented on Patrick Snowden's "QR code component" solution

    Hi Patrick!

    I've seen your github and you have a pretty good level, by the way here you have some errors mostly generated by the frontend mentor reports.

    To remove it is quite simple, you can change <div id="root"> by <div id="root" role="main"> (adding this attribute).

    Or the best option is to change <div id="root"> to <main id="root"> which is the tag to encompass the main content of your page since you should only have one <main> per page.

    This can be useful in different projects if they have the same errors. Do not forget to generate a new report.

    By the way a recommendation, always a REM for the font size, the reason is that it is a relative measure that depends on the font-size of the html that by default is 16px and at the time of modifying this it changes the one of all your page likewise many libraries are using REM for which if at some moment you call elements of one or another library they are going to adapt to the size of the letter that you have placed to your page. So at least that's my recommendation if you didn't know.

    I hope my comments are helpful :D!

    0
  • Solution screenshot
    • HTML
    • CSS

    Mobile-first Order Summary Component Using HTML, Scss, Flexbox

    7
    P
    Vanza Setia13,025 | Posted about 2 hours agocommented on Kingsley Agu's "Order summary component" solution

    Hi there! 👋

    You've got a lot of feedback from some people. So, I'm just going to give one or two pieces of feedback on this solution.

    • Firstly, I suggest using rem for font-size. It's important to make sure that it scales properly when the users change their default font size setting.
    • Secondly, I agree with @FluffyKas that only use aria-label if there's no visible text like icon buttons (e.g. Twitter "love" button, etc).

    Anyway, I decided to create a page where there's visible link text and aria-label. I tested it with Narrator in the Chrome browser, and here's the result.

    • Once the page is loaded, Narrator reads the visible text and ignores the aria-label.
    • After I tab (focus) to the link, Narrator reads the aria-label and ignores the visible text.
    • By the way, here's the HTML markup that I tested.
    <a href="/" aria-label="screen reader please read me">I just read the visible text</a>
    

    I suggest you to read this article by Josh Comeau about pixels and rems units. It's an amazing article that will tell you when to use px and when to use rem.

    So, that's it! Hope it's useful and sorry for the late reply. 😅

    1
  • Solution screenshot
    • HTML
    • CSS
    • JS

    Invoice app

    1
    P
    Po Rith220 | Posted about 3 hours agocommented on Ephraim Phil's "Invoice app" solution

    looks great! the only details I saw were the capitalization of the item statuses, commas in prices, and some strange spacing issues in the 'new invoice' button.

    With this, I may also check some things on the mobile view, whereas there seem to be some further spacing issues. All minor issues, overall... ~~

    I don't necessarily specialize in React, but the component structure looks good to me!

    0
  • Solution screenshot
    • HTML
    • CSS

    Stats preview card component

    1
    Saul560 | Posted about 3 hours agocommented on Iván Serrano's "Stats preview card component" solution

    Hello Iván! To achieve the right color in the image you should add .img_container::after{ content:""; background-color: var(--accent); opacity: 0.6; width:100%; }

    0
  • Solution screenshot
    • HTML
    • CSS

    3 column preview card component

    1
    Saul560 | Posted about 3 hours agocommented on Melissa Villegas's "3-column preview card component" solution

    Hello Melissa you did a fantastic job! The only thing I could say is to add a transition on the button's hover so it can look smoother. But in general, it looks terrific.

    0
  • Solution screenshot
    • HTML
    • CSS

    Order summary component responsive

    1
    Danilo Blas4,440 | Posted about 4 hours agocommented on JosueRoddy's "Order summary component" solution

    Josue, que tal!

    Tu proyecto me parece que esta bien aunque no soy muy fan de colocal la atribución con position absolute porque esta por encima de la tarjeta de resumen y hace que no se vea muy bien por lo menos en mi pantalla.

    Por cierto siempre limpia los errores que haya en el reporte que te generá frontend mentor, en este caso te falto colocar una etique <main> que funciona parecido al footer en contener varias etiquetas pero en este caso el <main> envuelve el contenido principal de toda la página por lo cual puedes cambiar <div class="card"> por <main class="card">.

    No se si ese errores solo lo tienes en este proyecto o en varios pero si es así masomenos ya sabes como solucionarlo apenas termine de de solucionarlo y subir tus cambios solo genera un nuevo reporte y deberían salirte 0 errores.

    Por ultimo para que el cambio de color, tamaño, etc se de en un elemento que le has puesto :hover, :active, etc. quieres que se de de una manera bonita, dale al elemento la propiedad transition por ejemplo: transition: background 200ms.

    Aplicaselo a los link y botones y verás que quedan geniales.

    Si tienes alguna duda comentamelo por aquí, suerte :D!

    0
  • Solution screenshot
    • HTML
    • CSS
    • JS
    • API

    Advice generator app solution

    1
    Danilo Blas4,440 | Posted about 4 hours agocommented on Chekkal Dai's "Advice generator app" solution

    Your project is fine, it is responsive and has a good functionality but I recommend you to fix the frontend mentor bug report.

    In your case you have two div inside your body, and you should put it in landmark tags or all inside the <main> tag. The reason is that the body is usually composed of different landmark tags and these in others. Among the landmark tags we have: <main>, <footer>, <nav>, <header>, etc.

    So to eliminate the errors at once make these changes:

    • Change <div class="container"> to <main class="container">.
    • Change <div class="attribution"> to <footer class="attribution">.

    With this update you should no longer have any problems. So don't forget to generate a new report to get 0 errors.

    Hope my comments are helpful :D!

    0
  • Solution screenshot
    • HTML
    • CSS

    nft-card-project

    1
    Danilo Blas4,440 | Posted about 4 hours agocommented on emmanuel's "NFT preview card component" solution

    Hi Emmanuel!

    You are uploading your links wrong!

    In the URL live project it should be:

    • https://maduka-102.github.io/nft-card-project/Nft-card-project/

    And the repository link is:

    • https://github.com/maduka-102/nft-card-project/tree/main/Nft-card-project

    That's why you can't see your projects or they load coming out that way because they are taking the root where the README.md capture is.

    Update your links so they can give you a better feeback and also frontend mentor can generate an error report.

    0
  • Solution screenshot
    • HTML
    • CSS

    HTML and CSS

    1
    Danilo Blas4,440 | Posted about 5 hours agocommented on Josue Garcia II's "QR code component" solution

    Hola Josue!

    Primero que nada no vayas tan rápido de proyecto en proyecto más bien verifica que todo ande bien antes de pasar de uno a otro.

    Por ejemplo frontend mentor te genera un reporte de errores de accesibilidad y html, se recomienda corregir todos los problemas antes de pasar a otro proyecto o se te irán acumulando, además la idea de los retos es que quede tu diseño final igual o lo mayor parecido al diseño propuesto de frontend mentor.

    Esto lo mencionó porque a tu reto le falta agregarle los colores que salen en style-guide.md, lo cual supongo que has revisado porque ahí tambien sale la tipografía, te recomiendo que actualices tú codigo con los colores correspondientes.

    Por cierto el <main> esta mal usado en este proyecto, deberías usarlo para englobar a todas las etiquetas que consideres que son parte central del contenido de la página en teoría debería ser toda la card. Y si colocas fuera de este el div con atribution por lo menos puedes cambiar el div por <footer> cosa que cumpliría un papel similar al <main> que es englobar las partes más importante de tu html. Luego de eso te recomiendo generar un nuevo reporte y ver que errores hay en este o si ya todos son corregidos, si tienes dudas puedes preguntar por aquí.

    0
  • Solution screenshot
    • HTML
    • CSS

    flexbox-css hover-effects

    1
    Danilo Blas4,440 | Posted about 6 hours agocommented on Juan Pablo Arias Duque's "NFT preview card component" solution

    Juanpa, hola!

    Lo primero que quería mencionarte es que no has agregado la tipografía supongo que se te ha pasado porque justo has agarrado los colores de style-guide.md así que eso esta pendiente.

    Lo otro es que le agregues siempre a tus imagenes display: block es por eso que cuando agregas el sombreado del apartado del hover te no te encaja porque las imagenes tienen un espaciado blanco abajo por defecto que se le quita haciendolas block.

    Otro detalle para eliminar los errores del reporte es que te falta la etiqueta <main> que es muy importarla tener dentro de tu body ya que esta se encarga de englobar el esquema principal de tu html así que agregala y que ocupe toda la card.

    Por ultimo puedes agregar transiciones con la propiedad transition a los elementos para que cambien de un estado a otro como los que le has agregado :hover para que el cambio no se brusco, sino sea más suave.

    Si tienes alguna duda consultame :D!

    0
  • Solution screenshot
    • HTML
    • CSS
    • JS

    Manage Landing Page - CSS Grid/flexbox

    1
    Khaizter70 | Posted about 6 hours agocommented on Andrew De La Fuente's "Manage landing page" solution

    you can change the color of svg by using the filter property here check this out https://stackoverflow.com/questions/22252472/how-to-change-the-color-of-an-svg-element

    0
  • Solution screenshot
    • HTML
    • CSS

    Single price grid component

    1
    Edgar0 | Posted about 7 hours agocommented on Kent O'Sullivan's "Single price grid component" solution

    Hello there Kent!

    I had the pleasure to review your solution and here below I leave some of my feedback:

    Looks great! Great use of semantic HTML and looks even better than the original design. There are some corners that are not adjusting, specifically the bottom corners. These should adapt to the border radius and they don't look round.

    0
  • Solution screenshot
    • HTML
    • CSS

    stats-preview-card HTML CSS

    2
    P
    Ahmed Bayoumi770 | Posted about 8 hours agocommented on nico's "Stats preview card component" solution

    Hey nico, You have an accessibility issues you need to fix.

    • Page should contain a level-one heading, Change h2 to h1 You should always have one h1 per page of the document.
    <h1 class="card__title">Get <span class="card__title card__title--color">insights</span> that help your business grow.</h1>
    
    • I suggest you put the status of the preview card component in the list item to add more semantics to your project. like this:
    <ul class="stats-preview__stats">
       <li>
          <span class="stat-number">10K+</span>
          <span class="stat-name">Companies</span>
       </li>
       <li>
          <span class="stat-number">314</span>
          <span class="stat-name">Templates</span>
       </li>
       <li>
          <span class="stat-number">12M+</span>
          <span class="stat-name">Queries</span>
       </li>
    </ul>
    

    I hope this is useful to you... Keep coding👍

    0
  • Solution screenshot
    • HTML
    • CSS

    stats-preview-card HTML CSS

    2
    Danilo Blas4,440 | Posted about 8 hours agocommented on nico's "Stats preview card component" solution

    Personally, in the mobile design I would add the following attribute.

    @media screen and (max-width: 1100px)
    .card {
      display: block;
      height: 90vh;
      max-width: 280px;
    }
    

    This way it doesn't expand too much and keeps the design but you should add more padding so it doesn't look so tight and maybe change the max-width but there you edit it to your liking.

    By the way don't use h2 if you don't have a h1 before, it is necessary to always have a h1 and only one per page since it is the title on the current page, the other h2, h3.... h6, working as subtitles of each other.

    With all that I think you can do that you no longer get errors in your report.

    I hope my comments are helpful. Good luck!

    0
  • Solution screenshot
    • HTML
    • CSS
    • JS
    • API

    Advice generator app - Solution

    2
    P
    Ahmed Bayoumi770 | Posted about 8 hours agocommented on Nathassia's "Advice generator app" solution

    Hey Nathassia, Congratulations on completing this challenge... You have some accessibility issues you need to fix.

    • Document should have one main landmark, Contain the component in <main>.
    <main>
       <div class="container">
          //...
       </div>
    </main>
    
    • An img element must have an alt attribute, to provide alternative information for an image if a user for some reason cannot view it.
    • This <div class="dice"></div> Should be button... Buttons are used for actions.

    I hope this is useful to you... Keep coding👍

    0
  • Solution screenshot
    • HTML
    • CSS
    • JS
    • API

    Advice Generator using HTML, CSS, JS

    1
    P
    Ahmed Bayoumi770 | Posted about 8 hours agocommented on Stefano Mainetti's "Advice generator app" solution

    Hey Stefano, It looks good!... You have some accessibility issues you need to fix.

    • Document should have one main landmark, Contain the component in <main>.
    <main>
       <div class="main-container">
          //...
       </div>
    </main>
    
    • Page should contain a level-one heading, You should always have one h1 per page of the document.
    • Buttons must have the discernible text, Set the attribute aria-label to describe this button.
    <button type="button" class="dice" aria-label="Advice generator"></button>
    

    I hope this is useful to you... Keep coding👍

    0
  • Solution screenshot
    • HTML
    • CSS

    QR Code Component HTML & CSS

    2
    Danilo Blas4,440 | Posted about 8 hours agocommented on Iván Serrano's "QR code component" solution

    Ivan!! Creo que te han dado un feedback bastante completo antes solo quería hacer enfasis en que cambies el <h3 class="info_container-title"> por <h1 class="info_container-title">, ya que es necesario que por página tengas 1 solo h1 y le podrías poner el atributo font-size: 1.3 rem para que encaje todo bien. El motivo del cambio es para que no tengas problemas de accesibilidad, frontend mentor siempre te genera un reporte con los posibles problemas de accesibilidad y HTML, y es nuestro deber corregirlos.

    Editalo y actualiza el reporte para que salga sin error, en lo personal buen proyecto siempre puedes agregarle el toque tuyo sobre todo si no te indican como debes hacer el proyecto pero en el diseño debemos hacerlo lo más similar posible ya que es lo que piden las empresas.

    Buenas suerte :D!

    0
  • Solution screenshot
    • HTML
    • CSS

    challenge #1 qr

    1
    P
    Ahmed Bayoumi770 | Posted about 8 hours agocommented on Yaseen Abdualhameed's "QR code component" solution

    Hey Yaseen, Congratulations on completing this challenge... You have some accessibility issues you need to fix.

    • Document should have one main landmark, Contain the component in <main>.
    <main>
       <div class="ppp">
          //...
       </div>
    </main>
    
    • Page should contain a level-one heading, Change <div>Improve your front-end skills by building projects</div> to <h1>Improve your front-end skills by building projects</h1>, You should always have one h1 per page of the document.

    This element <h1> represents the section headings.

    • I suggest you contain the attribution in <footer>.
    <footer>
       <div class="attribution">
          //...
       </div>
    </footer>
    
    • This should be contained in paragraph tag <p> <div>Scan the QR code to visit Frontend Mentor and take your coding skills to the next level</div>, to add more semantics to your project.
    <p>Scan the QR code to visit Frontend Mentor and take your coding skills to the next level</p>
    
    • An img element must have an alt attribute, to provide alternative information for an image if a user for some reason cannot view it. <img src="images/image-qr-code.png" alt="qr-code">

    I hope this is helpful to you... Keep coding👍

    0
  • Solution screenshot
    • HTML
    • CSS

    order component card

    2
    Danilo Blas4,440 | Posted about 8 hours agocommented on Leighton Buchanan-Cates's "Order summary component" solution

    Adding to the comment of @Ahmed Bayoumi

    The text "Order Summary" and "Annual plan" have the following color: color: hsl(223, 47%, 23%).

    By the way I don't recommend in this case to give percentage to your card because depending on how much the page is squeezed it will deform better use a fixed measurement:

    .container {
      width: 400px;
    }
    

    I mention this because I tried to center the card and it was deformed, now if you make that change you can center it vertically by adding to the body the min-height attribute

    body {
      background-image: url("file:///Users/user1/Desktop/projects/order-summary-component-main/images/pattern-background-desktop.svg");
      background-repeat: no-repeat;
      background-size: contain;
      display: grid;
      place-items: center;
      min-height: 100vh;
    }
    

    Although in this case I would recommend you to use flex because if you do it with grid consider the attribution of the size of the card to center them.

    By the way the background does not load you use a relative path to the root of the project instead of the absolute path.

    I hope my comments are helpful.

    0
Slack logo

Join our Slack community

Join over 100,000 people taking the challenges, talking about their code, helping each other, and chatting about all things front-end!