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
OverviewSolutions34Comments1749

All comments

  • Vanza Setia• 27,855

    @vanzasetia

    Posted

    Hi Jean! 👋

    Congratulations on finishing this challenge! 👏

    I recommend removing all the <div> elements. You do not need to wrap every element with a <div>. For example, you can make the <img> as a block element (display: block) and set max-width: 100% to prevent the image from overflowing.

    For the alternative text, I recommend telling "what the image is". In this case, it is a QR code that will redirect the users to frontendmentor.io. Also, you do not need to write the word "image" on the alternative text. Users will already know that it is an image. I recommend reading Quick tip: Using alt text properly - The A11Y Project.

    I hope this helps. Happy coding! 👍

    Marked as helpful

    0
  • Danny Poit• 20

    @dannypoit

    Submitted

    • How is my semantic HTML?
    • How is my implementation of clicking on the question to expand the answer, including the expanding height and changing plus/minus icon?
    • What improvements could be made to my CSS (assets/css/main.css)?
    • What CSS animations could be added/improved to make a better user experience?
    • How is my accessibility and what ways could it be improved?
    • Of course, any other thoughts/suggestions are welcome! Thanks!

    FAQ accordion

    3

    Vanza Setia• 27,855

    @vanzasetia

    Posted

    Hi, Danny! 👋

    Congratulations on finishing this challenge! 👏

    I recommend using the <details> and <summary> tags to create the accordion panels. Those elements are already having the close-and-open functionality. This way, you do not need to add JavaScript—which is good to have less code.

    If you want to keep using the <button> element, you need to add aria-expanded attribute to tell the state of the accordion—whether it is open or closed. Doing this allows users who use assistive technologies to know the state of the accordion.

    For progressive enhancement, you can try making sure that the buttons are removed when the JavaScript is blocked. You may not know that your JavaScript fails to arrive at the user's browser—Everyone has JavaScript, right?. If you want an example, you can visit inclusivedesignprinciples.org and try disable JavaScript.

    I hope this helps. Happy coding! 👍

    Marked as helpful

    1
  • William Miller Jr• 30

    @bijiyiqi2017

    Submitted

    How to make this semantic html?

    What areas needs improved, and how?

    How to do box shadows right as well as understand it well?

    Is there anything that I should study to make my second attempt an improved attempt?

    Any additional constructive feedback is welcomed. Thank you.

    My 1st Attempt ~ Creating a QR code component using flexbox

    2

    Vanza Setia• 27,855

    @vanzasetia

    Posted

    Hi, William! 👋

    Congratulations on finishing your first challenge! 👏

    Regarding the HTML, I recommend swapping <div id="qr-container"> with <main> tag. Remember that all HTML pages require to have one <main> tag. The reason is because that tag represents the main content of the page. It will be weird if there is no content on that page.

    There are more technical reasons to use that element. You can learn more about web accessibility on The A11Y Project.

    Also, I recommend using class attribute instead of id. id is used for anchoring or connecting the <label> element to the right <input> element. It is not recommended to use it for a hook in the stylesheet due to high specificity. Learn more: What the ID attribute is REALLY for

    I hope this helps. Happy coding! 👍

    Marked as helpful

    0
  • Vanza Setia• 27,855

    @vanzasetia

    Posted

    Hi, Yuki! 👋

    I found your solution in Frontend Mentor's newsletter. Nice solution! 👏

    It is great that you allow people to drag and drop to order todo items. Amazing! 👍

    It will be better if you add a focus indicator to any interactive elements. Right now, I can only see the focus indicator on the theme-switcher button and the "Clear Completed" button.

    Great solution! I hope this helps. 😄

    Marked as helpful

    1
  • Vanza Setia• 27,855

    @vanzasetia

    Posted

    Hi, JohanXTheKing! 👋

    Congratulations on completing this challenge! 🎉

    Also, good job on spending some time to write a README. 👍

    Here are some suggestions:

    • Decorative images should not have alternative text: The hero illustration is not a meaningful image. I recommend trying to remove the image. Then, try to check whether there is missing information or not. If there is no missing information, that image is a decorative image.
    • Do not use HTML markup for presentational purposes: Do not use a <br> element to move any text to the next line. Screen readers may announce <br> as "break" which creates unnecessary noise. Learn more: <br>: The Line Break element - HTML: HyperText Markup Language | MDN

    I hope this helps. Happy coding! 🙌

    0
  • Vanza Setia• 27,855

    @vanzasetia

    Posted

    Hi, Giovanni Brienza! 👋

    I do not know how to make the slider works with touch screen.

    I built the slider for this challenge using a third-party library, A11Y Slider - Library for simple and accessible sliders. By using that, I was able to make the slider responds to swipe gesture.

    Here are some suggestions:

    • Add alternative text to the logo: Logo is one of the ways for users to know the name of the site that they are visiting. So it is important to add alternative text to all logo images on the website.
    • Link and button are different: What do you think will happen after a user clicking the "Free Consultation" button? If the user will navigate to another page, then an anchor tag should be used. If it triggers an action, then the button element should be used.
    • Wrap the text with a meaningful element: For example, use a paragraph element to wrap the text. There should not be text in <span> and <div> alone.
    • Use a list element to list all the skills: "Graphic design", "Illustrations", and so on are a list of Amy's skills. I recommend using a list element. Also, I recommend making the pattern for each skill as a background image.
    • title attribute is not enough to label the buttons: I recommend having an alternative text on the arrow image that says, "Previous" or "Next". Or you can use an aria-label attribute to lable each button for the slider.

    I hope this helps. Happy coding! 😄

    0
  • Abhinav• 240

    @IamAbhiDev

    Submitted

    This is the second solution I am submitting for this challenge! Instead of specifying margin and padding for every element, I used CSS Grid to improve the layout and make it more responsive. It was a little challenging for me while making the responsive layout of the site as I am not used to making such layouts using CSS Grid but I tried my best. I would love to hear your feedback on this and help me improve my code as much as possible. Thank you for your time!

    Note - I had to darken the light text due to accessibility issues (contrast error)

    Responsive Four card feature section using CSS Grid

    1

    Vanza Setia• 27,855

    @vanzasetia

    Posted

    Hi, Abhinav! 👋

    Here are a few suggestions for improvements:

    • Remove the visually hidden heading: You do not need to add more content to the page. If it needs to be added, a visible heading should be preferred.
    • Invalid BEM: If you are following the BEM methodology for your class naming convention, section__card__content is BEE (Block Element Element). I recommend creating a new block (card) and then having an element for the new block (card__content).

    Great job on fixing the contrast issues! That is a good initiative.

    Nice work on the CSS. You are using clamp() for fluid typography and rem unit for font sizes. The grid layout works well too.

    I hope this helps. Happy coding! 😄

    Marked as helpful

    1
  • Vanza Setia• 27,855

    @vanzasetia

    Posted

    Hi, Vitor! 👋

    A couple of suggestions:

    • Fix invalid HTML: <h1> must not be a child element of <a>. You can use Caninclude to check whether an element can be inside another element.
    • No external links: You do not need to have target="_blank" to every link. Only use it if the users will lose something if the links do not open in a new tab. For example, the links that are required to open when the users fill in a form.
    • No extra element: You do need a <div> for the NFT card. You can use the <main> element instead. It is possible by making the <body> element as the flex container of the card instead of using the <main> element.
    • Do not use pixel unit for font sizes: Use rem or em instead of px for font sizes. Never use px unit. Relative units such as rem and em can adapt when the users change the browser's font size setting. Learn more — Why you should never use px to set font-size in CSS and Why font-size must NEVER be in pixels
    • Unitless line-height: Always use unitless numbers for line-height values to avoid unexpected results. Learn more — line-height - CSS: Cascading Style Sheets | MDN

    I hope my suggestions help you. Have fun coding! 😄

    Marked as helpful

    1
  • Abdelrahman• 90

    @Dribbz

    Submitted

    Hi there 👋🏼, I’m Dribbz and this is my solution for this challenge. 🚀

    🛠️ Built With:

    ⚡️HTML5 ⚡️Vanilla CSS

    Any Suggestions On how to improve my code is Welcomed 🙌🏼

    Thank you !✌️

    Single price grid component

    #accessibility

    1

    Vanza Setia• 27,855

    @vanzasetia

    Posted

    Hi, Dribbz! 👋

    A couple of suggestions:

    • No extra element: You do not need an extra <div> element—<div class="grid-container">—to wrap all the card sections. You can use the <body> element as the grid container of the card or the <main> element. Then, use the <main> element as the .grid-container.
    • Use appropriate element: The content below the "Why Us" text is a list of reasons, so you should use a list element.
    • Remove default styling: By default, <p> elements will have font-size: 1rem. You can remove font-size: 1rem from the .description styling.

    I hope my suggestions help you. Have fun coding! 😄

    Marked as helpful

    0
  • Joel Leon• 100

    @iJoel23

    Submitted

    Hey everyone 🐣!

    I found difficult the hover over the first image. If there's a better way to do it, would be very helpful to know.

    NFT preview card component

    #animation

    1

    Vanza Setia• 27,855

    @vanzasetia

    Posted

    Hi, Joel Leon! 👋

    Before taking care of the hover effect, I recommend getting the HTML right first by wrapping the Equilibrium image with an anchor tag. It has interactivity, so it should be wrapped by an interactive element.

    Now for the hover effect, I recommend using a pseudo-element and background properties. So, there is no HTML markup for the overlay and the eye icon.

    You can start by creating the pseudo-element from the anchor tag. Then, use the pseudo-element to show eye icon by using background-image and background-position. For the background color, use rgba or hsla to control the opacity of the color.

    For your reference, you can see @IlnaraAckermann's solution.

    Frontend Mentor | Css3, HTML5, flexbox, pseudo-class and pseudo-elements coding challenge solution

    I hope this helps. Happy coding!

    0
  • Dumpling• 10

    @RVilums

    Submitted

    This is my solution for the QR Code Component challenge from Frontend Mentor. The challenge was to create a QR code component that displays a QR code image along with a title and description. The component needed to be responsive and match the provided design preview.

    I built the QR code component using HTML, CSS, and Flexbox.

    I thoroughly enjoyed working on this challenge as it allowed me to apply my HTML and CSS skills to create a practical and visually appealing component. I found the experience valuable in terms of improving my frontend development abilities.

    I would greatly appreciate any feedback or suggestions for improvement. Thank you for taking the time to review my solution!

    QR Card Component / Flexbox

    1

    Vanza Setia• 27,855

    @vanzasetia

    Posted

    Hi, Dumpling! 👋

    Here are some suggestions:

    I hope this helps. Happy coding!

    Marked as helpful

    1
  • Rodriguez Facundo Gabriel• 10

    @RodriguezFacundoG

    Submitted

    Is it good for you not to use media queries? Does it still feels like a good design? Since it's a simple application I preferred to use rem as unit of measurement, and to have the html font-size attribute linked by a calc() with de vw, so, as the screen widens, all of the rem attached attributes are being modified as well.

    I've used HTML and CSS with Responsive Design

    1

    Vanza Setia• 27,855

    @vanzasetia

    Posted

    Hi, Rodriguez Facundo Gabriel! 👋

    Here are some suggestions for improvements:

    • Alternative should describe the image: Alternative text is not for telling the users the image fails to load. It should describe the purpose of the image. Describe what is the image about (QR code).
    • Do not skip heading levels: Start from <h1>. Heading levels must always be in order to give structure to a page.
    • Do not change the <html> or the :root font size: It can cause huge accessibility implications for those users with different font sizes or zoom requirements. Grace Snow explains the issue clearly—Should I change the default HTML font-size to 62.5%?—and Joshua Comeau also does not recommend that approach—The Surprising Truth About Pixels and Accessibility: should I use pixels or rems?.
    • Remove extra element: You do not need <div class="component">. You can use the <body> element as the flex container of the card. Then, use the <main> element as the card component. Also, you do not need <div class="attribution">. You can just use the <footer>.
    • Use padding to prevent child elements from touching the edges of the parent element: You can set padding on the card to push the element toward inside the card. For the image, you can remove the margin: 6% 6% 0 6%; and set the width to 100%. The same goes for the <div class="textContainer">, remove the margin and the div element itself.

    I hope this helps. Happy coding! 👍

    0
  • Vanza Setia• 27,855

    @vanzasetia

    Posted

    Hi, C4rlos! 👋

    One suggestion is to remove the "Picture of" phrases from the alternative text. Alternative text for images should not contain any words related to "image" (such as picture, photo, logo, icon, graphic, avatar). It is already an image element (<img>) so the screen reader will pronounce it as an image.

    Everything else on the HTML looks good to me. Great job!

    Marked as helpful

    0
  • Karan Niroula• 50

    @eatsNotMe

    Submitted

    This was my first time making a responsive website and using a grid layout and media queries. This website still has several flaws. Please view this website from a mobile phone perspective so that you can see the problem. If you know how to fix this issue, I will be very glad.

    Huddle Landing Page | HTML & CSS

    1

    Vanza Setia• 27,855

    @vanzasetia

    Posted

    Hi, Karan Niroula! 👋

    Could you please describe the problem? At what screen size does the problem happen?

    I recommend using a code-formatter. This way, your code base will have a consistent format, which makes it easier to read the code. I suggest using Prettier as your code-formatter.

    Prettier · Opinionated Code Formatter

    Here are a few suggestions for improvements:

    • Use landmark element: Wrap the whole content with a landmark element. The social media links can be wrapped with a <footer>. The rest of the content should be inside the <main> landmark.
    • <img> must have alt attribute: Each of <img> element must have alt attribute.
    • Make sure the alternative text includes the image's text: For images containing text like the Huddle logo should have an alt value of “Huddle”. Reference: Checklist - The A11Y Project #for-images-containing-text-make-sure-the-alt-description-includes-the-images-text
    • Avoid legacy element: You should not use HTML to make the text bold by using <b> element. Also, <b> should be avoided whenever possible.
    • Each page must have one <h1>: The <h1> element is used to define the main heading of a page. The "Build The Community Your Fans Will Love" should be wrapped by a <h1>.

    You need to make sure the HTML is right first. I know that you are learning CSS Grid. But, you should get the HTML right because the HTML impacts the way users of assistive technologies interact with the page. Also, if the CSS fails to load the page will still make sense because of the correct user agent stylesheet.

    I hope this helps. Happy coding! 👍

    Marked as helpful

    0
  • Vanza Setia• 27,855

    @vanzasetia

    Posted

    Hi, Mikhail! 👋

    For the circles, I recommend showing them with pseudo-elements. You can see Grace's solution for your reference. Her solution does not require a media query to position the circles.

    Frontend Mentor | Profile card with pseudo backgrounds and accessible list coding challenge solution

    Here are a few suggestions for improvements:

    • Alternative text should not describe the look of the picture: Do not describe what the picture looks like, i.e. "a graphic of bright blue bird" as the alternative text of the Twitter icon. Instead, describe what the image content is and what it does, such as "Twitter" in this case. In this case, the photo's alternative text should be the name of the person.
    • No empty elements: Remove the <header class="card__header"></header>. Then, use the <img> to show the background pattern of the card.
    • Each page must have one <h1>: The <h1> element is used to define the main heading of a page. In this case, replace <p class="card__content-name">Victor Crest</p> with <h1>.
    • Do not use pixel unit for font sizes: Use rem or em instead of px for font sizes. Never use px unit. Relative units such as rem and em can adapt when the users change the browser's font size setting. Learn more — Why you should never use px to set font-size in CSS

    I hope this helps. Happy coding! 👍

    Marked as helpful

    1
  • Kevin H.• 150

    @kevinx9000

    Submitted

    My feedback and difficulties mostly came from responsive design adjustments with Grid and rem units. Here's where I need feedback:

    • Use of CSS Grid; I did a mobile-first workflow, and didn't implement Grid until the larger breakpoints; seems like there may have been a better way to use this and write less CSS for the breakpoints
    • Use of rem units; mobile-first was fine, but then going to large screens made everything seem too small, so I made a lot of manual size adjustments in the breakpoints; again, feels like there would have been a better way to master rem (and maybe em) units so that the size adjustments happen automatically for breakpoints

    Otherwise, I welcome feedback about any other observations. Thank you!

    Single price grid component using Grid, rem units, media queries

    1

    Vanza Setia• 27,855

    @vanzasetia

    Posted

    Hi, Kevin! 👋

    My feedback for the two mentioned points:

    • You only need to use one media query to switch the card from the one-column layout to the grid layout. You do not need to define grid-template-rows: auto; on the .container. That is already the default behavior. After that, you do not need to define the grid-column: span 1; on the second and third grid items. Try removing them and see if there is any difference.
    • You do not need to adjust the font size for larger screen sizes. You do not need media queries for the font sizes. On all screen sizes, the font sizes are the same. So, you only need a media query to switch the card's layout.

    People with large screen sizes will zoom in or increase the zoom level. So you do not need to worry about making adjustments for large screen sizes such as those above 1440px.

    Here are a few suggestions for improvements:

    • No extra element: Use the <main> element as the .container instead of having another <div> element.
    • One max-width: You only need to specify one max-width for the .container. You should put the max-width as the base styling of the .container.
    • Do not use pixel unit for font sizes: Use rem or em instead of px for font sizes. Never use px unit. Relative units such as rem and em can adapt when the users change the browser's font size setting. Learn more — Why you should never use px to set font-size in CSS
    • Do not change the <html> or the :root font size: It can cause huge accessibility implications for those users with different font sizes or zoom requirements. Grace Snow explains the issue clearly—Should I change the default HTML font-size to 62.5%?—and Joshua Comeau also does not recommend that approach—The Surprising Truth About Pixels and Accessibility: should I use pixels or rems?.
    • Do not specify default styling: Remove font-size: 1rem from the <body> styling. It is already the default styling.

    I hope this helps. Happy coding! 👍

    Marked as helpful

    0
  • Vanza Setia• 27,855

    @vanzasetia

    Posted

    Hi, Alan C. Gabales! 👋

    Here are a few ways you can do to improve this solution:

    • All the page content should live inside landmark elements: Users of assistive technology can navigate through landmark elements. This will help them quickly navigate a website or application. In this case, the children of the <body> should be landmark elements; <main> for the card, <footer> for the attribution (if you want to have an attribution).
    • Wrap the text with a meaningful element: For example, use a paragraph element to wrap the text. There should not be text in <span> and <div> alone.
    • Use a list for the statistic: Use a list element for the company's statistic.
    • Do not use pixel unit for font sizes: Use rem or em instead of px for font sizes. Never use px unit. Relative units such as rem and em can adapt when the users change the browser's font size setting (as @Sachin0000000 mentioned earlier). Learn more — Why you should never use px to set font-size in CSS
    • Use <picture> element for the card's image: You can make the image responsive—which means switching from the mobile to the desktop version—by using the <picture> element. Then, you can use the mix-blend-mode property to create the purple overlay.

    I hope this helps. Happy coding! 👍

    0
  • Vanza Setia• 27,855

    @vanzasetia

    Posted

    Hi, Emanuel Bonardo! 👋

    Here are some suggestions for improvements:

    • No <div> elements: Remove all the <div> elements. The styling of the .container and the .card__container can be merged and put on the <main> element. You do not need to wrap each element with a <div> element.
    • Use owl selector: For spacing between elements inside the card, you can use the owl selector (* + *). For example, you can have .card > * + * { margin-top: 1em }.
    • Alternative text should describe the image: Alternative text for images should not include any words that are related to the word "image". The semantic meaning of the <img> element tells assistive technologies to pronounce it as an image. So, you should describe what is the image and the purpose of the QR code.

    I hope this helps. Happy coding! 👍

    Marked as helpful

    2
  • Vanza Setia• 27,855

    @vanzasetia

    Posted

    Hi, jcastro! 👋

    For the fluid position of those circles, I recommend showing them as pseudo-elements. You can see Grace's solution for your reference.

    Frontend Mentor | Profile card with pseudo backgrounds and accessible list coding challenge solution

    I have some suggestions to improve your solution.

    • Use <div>: Do not use <article> or <section> for all content. Use <article> for an article such as blog post content. It is okay to use <div> to separate content for styling purposes. Also, those two elements have no meaning if they are not labeled properly using the ARIA attribute—WebAIM: HTML Semantics and Accessibility Cheat Sheet.
    • Use a list element: Use <ul> for the statistic. Also, the number should not be a heading.
    • No extra element: Use the <main> element as the wrapper instead of using <section class="wrapper">.
    • Do not specify default styling: Remove width: 100% from the <body> styling. It is already the default styling.

    I hope this helps. Happy coding! 👍

    0
  • MeghaS4831• 30

    @MeghaS4831

    Submitted

    Hi!

    This is my second challenge and I'm super excited to explore further :D In this challenge, I have aimed to use semantic elements wherever I felt it was needed. Hope it is correct.

    I also wanted to understand if there is a way to change the svg images background using CSS? For instance, I used an svg file as <body> background but I wanted the colors to be different than ones currently in use.

    Feel free to share your best practices to enhance the code efficiency! :)

    Profile card using Flex and CSS Positioning

    #accessibility

    2

    Vanza Setia• 27,855

    @vanzasetia

    Posted

    Hi, MeghaS4831! 👋

    For those circles, I recommend showing them as pseudo-elements. You can see Grace's solution for your reference.

    Frontend Mentor | Profile card with pseudo backgrounds and accessible list coding challenge solution

    This is another way to help you position those circles.

    If you want to change the colors of those circles, you need to change the value of the fill property in the SVG file. But, I think that can be tricky because the colors are gradient colors.

    Some suggestions:

    • Alternative text should be in a readable format: Alternative text for images should not be hyphenated.
    • Decorative images should not have alternative text: Not every image needs alternative text. This will tell the screen reader to skip over the decorative images. As a result, it saves screen reader users time navigating the page. For your information, decorative images are images that do not add any information and serve only aesthetic purposes.
    • No inline styling: Move all the styling to the external stylesheet—style="font-size: 18px; font-weight: 700". Separating the HTML and the CSS can make it easier to maintain the code.
    • Do not use pixel unit for font sizes: Use rem or em instead of px for font sizes. Never use px unit. Relative units such as rem and em can adapt when the users change the browser's font size setting. Learn more — Why you should never use px to set font-size in CSS

    Avoid id selectors: Do not use id selectors for styling. There are two reasons for not using ID’s to style content:

    • They mess up specificity because they are too high (the most important reason).
    • They are unique identifiers. So, they are not reusable on the same page.
    • Learn more — What the ID attribute is REALLY for

    I hope this helps. Happy coding! 👍

    Marked as helpful

    1
  • Tharun Raj• 400

    @Code-Beaker

    Submitted

    Hello, I have a doubt on centering the .wrapper on the screen for that, I used

    The .container is the parent of .wrapper

    <div class="container">
<div class="wrapper">
</div>
</div>

    .container {
position: relative;
height: 100vh;
}

.wrapper {
display: flex;
flex-direction: row;
max-width: 800px;
max-height: 400px;
padding: 30px;
position: absolute;
inset: 0;
margin: auto;
}

    3-Column Preview Card with HTML&CSS

    2

    Vanza Setia• 27,855

    @vanzasetia

    Posted

    Hi, CodeBeaker! 👋

    Before getting into the styling to put the cards in the middle of the page, I recommend fixing the HTML markup first.

    • No extra elements: You only need one <div> which is to wrap all the cards. You can remove the <div class="container">. You can use the <body> element instead.
    • Landmark element: You should swap the <div class="wrapper"> with <main>. Users of assistive technology can navigate through landmark elements. This will help them quickly navigate a website or application.
    • Replace all the <h1> with <h2>: There should not be more than one h1 on a page. Many <h1> elements mean many titles which can confuse the users, especially the screen reader users.

    Then, to put all the card in the middle of the page, you can make the body element a flex or grid container of the element that wraps all the cards—<main>.

    Another suggestion for styling, you do not need to use absolute positioning on the wrapper.

    I hope this helps. Happy coding!

    Marked as helpful

    1
  • Ana• 90

    @akajfes

    Submitted

    I tried to use Sass for this and will try to use NPM instead of VSCode extensions build and watch the code. I added a thank you message after the validation too. I'm wondering if there's a way to condense my Javascript code.

    Form validation with Javascript and styled with Sass

    #sass/scss

    1

    Vanza Setia• 27,855

    @vanzasetia

    Posted

    Hi, Ana! 👋

    Here are some suggestions for improvements.

    • Wrap the text with a meaningful element: For example, use a paragraph element to wrap the text including the error message. There should not be text in <span> and <div> alone.
    • Progressive enhancement: Add the novalidate attribute to the <form> through JavaScript. Then, add required attribute to all <input> elements. This way, if the JavaScript fails to load or the users disable JavaScript, the native form validation will be running.
    • Do not use legacy or deprecated elements: Use <button type="submit"> instead of <input type="submit">. Submit input is legacy.
    • One declaration for @import: You only need to write @import once on the stylesheet. As a side note, using <link> tags are recommended for better performance.
    • Do not use pixel unit for font sizes: Use rem or em instead of px for font sizes. Never use px unit. Relative units such as rem and em can adapt when the users change the browser's font size setting. Learn more — Why you should never use px to set font-size in CSS
    • Keep the .gitignore: This can prevent your repository from getting filled with .DS_Store files.

    I hope this helps. Happy coding! 👍

    0
  • Vanza Setia• 27,855

    @vanzasetia

    Posted

    Hi, Marcia! 👋

    Congratulations on finishing this challenge! It is great that you do this project by yourself. 👍

    I have one suggestion:

    No <br> elements: Do not use <br> element to break text. You should make the <span> as a block element. Learn more about accessibility issues that can happen when using <br> — <br>: The Line Break element - HTML: HyperText Markup Language | MDN #accessibility_concerns

    I hope this helps. Happy coding!

    Marked as helpful

    0
  • P
    Lo-Deck• 1,360

    @Lo-Deck

    Submitted

    Hi Front-end Mentor community. Here is my solution for Social-proof-section.

    I used grid to display the website in the desktop version. But I have some question about it. Should I use the rem measures for margin, padding, etc, or I'd better use measure like % or vw instead.

    What is the best to use with grid?

    Can you give me some tips about it. Thank you all;

    Social proof section

    1

    Vanza Setia• 27,855

    @vanzasetia

    Posted

    Hi, Lo-Deck! 👋

    For margin and padding, I recommend using rem. Most of the time, it is better to have a consistent space.

    You can use fluid space by using viewport units and clamp() function. You need to make sure the minimum and the maximum value using rem unit. If you use a viewport or percentage unit, the space will keep changing.

    This tool can help you generate fluid space — Fluid Responsive Design | Utopia

    In this case, I think it is better to use grid-template-areas. You can divide it into three areas. The first area is the "10,000+ of our users love our products" and the below paragraph. The second area is the ratings. The third area is the testimonials.

    Also, you may only need four columns instead of 12 columns. For the desktop layout, two columns for the first area and the second area. The testimonials take all four columns.

    Here are some suggestions for improvements:

    I hope this helps. Happy coding! 😄

    Marked as helpful

    2