Skip to content
OverviewSolutions
34
Comments
1742
Vanza Setia
@vanzasetia

All comments

  • Vanza Setia•27,775
    @vanzasetiaPosted 11 months ago

    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
    @dannypoitSubmitted 11 months ago
    • 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,775
    @vanzasetiaPosted 11 months ago

    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
    @bijiyiqi2017Submitted 11 months ago

    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,775
    @vanzasetiaPosted 11 months ago

    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,775
    @vanzasetiaPosted about 1 year ago

    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,775
    @vanzasetiaPosted about 1 year ago

    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,775
    @vanzasetiaPosted over 1 year ago

    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
    @IamAbhiDevSubmitted over 1 year ago

    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,775
    @vanzasetiaPosted over 1 year ago

    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,775
    @vanzasetiaPosted over 1 year ago

    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
    @DribbzSubmitted over 1 year ago

    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

    1
    Vanza Setia•27,775
    @vanzasetiaPosted over 1 year ago

    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
    @iJoel23Submitted over 1 year ago

    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

    1
    Vanza Setia•27,775
    @vanzasetiaPosted over 1 year ago

    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
    @RVilumsSubmitted over 1 year ago

    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,775
    @vanzasetiaPosted over 1 year ago

    Hi, Dumpling! 👋

    Here are some suggestions:

    I hope this helps. Happy coding!

    Marked as helpful
    1
  • Rodriguez Facundo Gabriel•10
    @RodriguezFacundoGSubmitted over 1 year ago

    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,775
    @vanzasetiaPosted over 1 year ago

    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