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
OverviewSolutions20Comments24

All comments

  • P
    Dave Quah• 670

    @Milleus

    Posted

    Greetings! 👋 Congratulations on completing the challenge.

    I have some suggestions for improvements:

    IMAGE ALT TEXT

    • Consider having a more meaningful alt text for the QR code image. For example, <img src=“…” alt=“QR code to frontendmentor.io” />.

    HEADING STRUCTURE

    • Consider having a unique h1 per page that describes what the page is about. Having the right heading levels would help screen reader users and also search engines. More details about heading structure here.

    LANDMARKS

    • The <main> landmark is missing, consider renaming the <div> to <main>. Landmarks help to define major sections of your page and can help screen reader users a great deal. More details about landmarks here.

    I hope this helps and happy coding! 😊

    0
  • P
    Dave Quah• 670

    @Milleus

    Posted

    Greetings! 👋 Congratulations on completing the challenge.

    I have some suggestions for improvements:

    IMAGE ALT TEXT

    • Consider having a more meaningful alt text that explains the purpose of the QR code image. For example, <img src=“…” alt=“QR code to frontendmentor.io” />.

    HEADING STRUCTURE

    • Consider having a unique h1 per page that describes what the page is about. More details about heading structure here.

    I hope this helps and happy coding! 😊

    0
  • P
    Dave Quah• 670

    @Milleus

    Posted

    Greetings! 👋 Congratulations on completing the challenge.

    I have some suggestions for improvements:

    • Consider using min-height: 100vh in the body instead of height: 100vh. This is because height: 100vh may cause the component to be cut off on smaller screens.

    Explanation:

    • If we set height: 100vh on the body, then body will always be 100% viewport’s height regardless of the content. So if the content is more than 100vh, it would be cut off.
    • If we replace this with min-height: 100vh and leave height as its default value, then the body’s height would minimally be at 100vh but can still grow beyond 100vh depending on the content.

    I hope this helps and happy coding! 😊

    Marked as helpful

    0
  • P
    Dave Quah• 670

    @Milleus

    Posted

    👋 Hello!

    Congratulations on completing the challenge. 🥳

    There are a few things that I think would make this even better:

    • Use only one <h1> per page, for accessibility and SEO. In this case I'd see "Sedans", "SUVs" and "Luxury" of the same logical level, so I'd approach this by adding a <h1> with a screen reader only class (visually hidden but still picked up by screen readers), and have "Sedans", "SUVs" and "Luxury" as <h2>. Here's a good video by Kevin Powell sharing more about headings.
    • In the footer, to be more semantically correct, use <p> instead of <div> for texts.
    • For the rounded borders, an alternative approach is to set border-radius: 8px and overflow: hidden on the parent. This could make things a little easier as the border-radius would not need to be updated in the media query.
    • For the anchor elements CSS, because the font-size isn't explicitly set and there's a fixed pixel height and width set, there will be text overflowing out of the element when the base font-size increases (i.e. someone who increased font size on their device). Most cases of fixed widths or heights could be replaced with min-width or max-width etc, so that it scales better. Another approach would be to use padding instead. I'd also recommend using em or rem units instead of px for better accessibility.

    Nice work and keep up the good job! I hope this helps and happy coding! 😊

    Marked as helpful

    0
  • Mochimooo• 50

    @mochimooo

    Submitted

    Hi Frontend Mentor peers:

    1. Is there a better way to write the hover effect than how I wrote it in my code?
    .button:is(:hover,:focus){
    background-color:hsl(245, 75%, 52%, 80%);
    cursor: pointer;
}

.hover:is(:hover,:focus){
    cursor:pointer;
    color:hsl(228, 45%, 44%);
    font-weight: var(--fw-bold);
}

.hover-1:is(:hover,:focus){
    cursor:pointer;
    color: black;
    font-weight: var(--fw-bold);
}
    1. How can I anchor the footer note to the bottom of the page? I tried using `.bottom{position: fixed; bottom: 5px;' and it would cause the footer note to overlap onto the summary card. 🤷‍♀️

    Thank you.

    Order Summary Component Challenge

    1

    P
    Dave Quah• 670

    @Milleus

    Posted

    👋 Hello!

    I think using :is is fine, but I would probably move the cursor: pointer to the base element (non-hover) instead.

    More importantly though, it would be a lot better if the proper interactive HTML elements, i.e. <button> or <a>, were used instead of <div>, <u> and <b>. Using the proper elements would help with accessibility greatly - able to navigate on with keyboard, pressing "Space" or "Enter" triggers the element, screen reader would announce that it is a clickable button or link, etc.

    As for how to anchor the footer note, my approach was to use CSS grid. I'd declare 3 rows (1fr, auto, 1fr) and have the <main> in row 2 and <footer> in row 3. Row 1 is an empty row to help keep <main> centered perfectly (thus why it is 1fr, similar to row 3).

    I hope this helps and happy coding!

    Marked as helpful

    1
  • P
    Dave Quah• 670

    @Milleus

    Posted

    👋 Hello!

    This is a nice solution - I like the use of HTML elements and CSS custom properties.

    I think a few things that would make this even better:

    • The "attribution" as a <p> instead of a <div>, and if it could fit in the same viewport without scrolling.
    • Addressing the accessibility concerns with Safari browser for list-style: none
    • Perhaps consider utility classes for font size, weight and color instead of declaring them with headings (h1, h2). This gives more flexibility as most times headings aren't always styled the same.

    Really nice! I look forward to seeing more solutions from you. Happy coding!

    Marked as helpful

    0
  • P
    Alper• 1,010

    @adonmez04

    Submitted

    Hi, everyone.

    This is my second solution to the project (the mobile first again). I got a huge feedback from Grace Snow and fixed some bugs according to them. I tried to solve some problems with responsive design. I guess this solution is a progress. I know there are lots of issues still here. I hope I can find and fix them. Thanks...

    Any comments, critique, advice is greatly appreciated. For those of you reading this, have a nice day!

    3-column-preview-card-component-v2.0

    1

    P
    Dave Quah• 670

    @Milleus

    Posted

    👋 Hello!

    I think one thing that could be improved on is the heading levels. I'd view "Sedans", "SUVs" and "Luxury" as the same heading, but I also understand that we do not want multiple <h1>.

    I think it would be better to declare a <h1> but set it for screen readers only (meaning not visible but screen reader would still pick it up), and keep "Sedans" as a <h2> with "SUVs" and "Luxury". E.g.

    // CSS
.sr-only {
  position: absolute;
  top: auto;
  height: 1px;
  width: 1px;
  overflow: hidden;
  clip: rect(1px, 1px, 1px, 1px);
  white-space: nowrap;
}

// HTML
<h1 class="sr-only">Learn more about our cars</h1>
...
<h2>Sedans</h2>
<h2>SUVs</h2>
<h2>Luxury</h2>

    Another suggestion is that for images that are decorative, the alt should be alt="" instead of alt="#", or alternatively you could also set aria-hidden="true" to hide it from screen readers.

    Some useful resources:

    I hope this helps and happy coding~

    Marked as helpful

    1
  • P
    Dave Quah• 670

    @Milleus

    Posted

    👋 Hello! Congrats on completing the challenge.

    Tailwind actually comes with preset font-sizes, I'm curious why not use those instead? e.g. text-xs which is 0.75rem (12px), text-sm which is 0.875rem; (14px), etc?

    Alternatively, font-size could be overwrote with your own set of sizes via tailwind.config.js.

    Also, one tiny trick that could help with writing lesser code is to use the gap (flex and grid) or the space-y and space-x utilities. They help to add gaps or margin between child elements so you wouldn't need to keep adding margins to every child element 😊

    Hope this helps and happy coding~

    Marked as helpful

    0
  • Bel030• 10

    @Bel030

    Submitted

    I had trouble centering the div without affecting the format of the text and image, along with centering the div itself rather than being located on the top-left corner of the body. This was resolved by implementing a margin to the div.

    QR code solution

    1

    P
    Dave Quah• 670

    @Milleus

    Posted

    👋 Hello! Congratulations on completing your first challenge.

    Typically you'd want to add a min-height: 100vh to the body so that the minimum height stretches to match the viewport. After that we could use Flexbox or Grid to help with centering. E.g.

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

    or with grid

    body {
  display: grid;
  place-items: center;
}

    I'd suggest reading up more about Flexbox and Grid. MDN website is a good source for learning. There are also Flex/Grid game websites to help with learning.

    Other than the issues highlighted in the accessibility report, one other thing that could be improved is to change the <div class="attribution" to a <p> tag instead as it would be more semantically correct.

    Hope this helps and happy coding~

    Marked as helpful

    1
  • Kierro DeCampanille• 130

    @Kirp

    Submitted

    Had a rough time with properly placing the image and trying to figure out a non js solution to the accordion effect. In the end it felt really wonky to have a form just to record if its clicked so I just went with using js.

    FAQ accordion card solution

    1

  • Nikolas Papenfus• 380

    @NPM0486

    Submitted

    Are the class names appropriate? Did I give too many div tags? I had a problem with the hover on the photo, I currently made a div that is above the photo, but in the plans I had to use :after but it didn't work so I went to plan B.

    Simple solution using Flex and Grid

    1

    P
    Dave Quah• 670

    @Milleus

    Posted

    Hi Nikolas, congrats on completing the challenge 🥳

    There seem to be an issue where when the mouse is at the center of the picture (where the invisible "eye" is), the overlay and eye disappears. I believe this is because in CSS the effect triggers on .overlay:hover, but when the mouse is in the center it is hovering over the invisible <img class="view"> instead of the <div class="overlay">.

    One way to fix it is to set pointer-events: none to the <img class="view">, https://developer.mozilla.org/en-US/docs/Web/CSS/pointer-events.

    Another way to fix this is to remove <img class="view"> and add it as a background-image to the overlay instead.

    I hope you wouldn't mind if I share my solution with you. It uses background-image as well as :after - https://www.frontendmentor.io/solutions/nft-preview-card-component-rJRIRaSVc

    Hope this helps and happy coding!

    Marked as helpful

    0
  • Martin Amankwaa• 30

    @MartyOfMCA

    Submitted

    Could anyone kindly explain you to what I'm doing wrong in setting the font family for my title (content-title in my markup file) to Fraunces.

    I humbly welcome suggestions on any other areas I missed. Thank you!

    HTML & CSS Simple Layout

    #bem

    1

    P
    Dave Quah• 670

    @Milleus

    Posted

    Hello,

    The font import URL is incorrect. Go to the font links and select the font weights/styles, a side window will pop up with the selected fonts and guide you on how to import the fonts.

    • https://fonts.google.com/specimen/Fraunces
    • https://fonts.google.com/specimen/Montserrat

    The import should look something like this

    @import url('https://fonts.googleapis.com/css2?family=Fraunces:opsz,[email protected],700&family=Montserrat:wght@500;700&display=swap');

    Hope this helps!

    Marked as helpful

    0
  • Ranjana Mukhia• 150

    @ranjanamukhia

    Submitted

    I want the feedback on following things:- 1.Please let me know what can be done to make it responsive as iam not sure how the white card having blue-background and persons photo should come up in smaller screen size than 340px.

    profile card with html , css mobile and desktopversion

    1

    P
    Dave Quah• 670

    @Milleus

    Posted

    Hi @ranjanamukhia 👋

    Congrats on completing the challenge 🎉

    To your question on how to fix accessibility issues:

    • The page needs a level-one heading, meaning it needs a <h1> element. More information here. If the existing design does not have an appropriate text to be a h1, you can add your own h1 title and make it visibly hidden but still picked up by screen readers.

    • The <div class="attribution"> needs to be within a landmark, landmark refers to elements such as <header>, <nav>, <main>, <footer>. There are several ways to fix this, either move the <div> into the <main>, wrap it within a <footer>, or changing the <div> to a <footer> since it makes sense in this case. More information here.

    Hope this helps, happy coding!

    Marked as helpful

    0
  • Kariman Medhat• 170

    @KarimanMedhat

    Submitted

    I faced the hardest time in this project, I managed to make it looks good on desktop but I filed on mobile and I tried Flex-box and grid both of them didn't work. Have any idea what should I do?

    Social proof challenge

    1

    P
    Dave Quah• 670

    @Milleus

    Posted

    Hi there!

    I noticed that you used a lot of position: absolute and transform: translate to shift elements. This makes things a lot more complicated to work with, I would suggest not to use this excessively.

    The layout can actually be achieved with CSS grid and flex. If you're comfortable with watching a video guide to this challenge, here's a good video on how Kevin Powell approached this.

    In my opinion, this challenge is one of the harder newbie challenges. I hope this does not discourage you. Keep at it and happy coding~

    Marked as helpful

    0
  • Pratyusha Majumder• 280

    @FloraBloomblue

    Submitted

    This is my second challenge. The hovering part , I tried to keep it in simple css.

    "transform: translate(-50%, -50%); -ms-transform: translate(-50%, -50%);" --- this one I saw but I tried to use the basic css for the hovering part because I am a beginner and didn't fully understood the "transform" part. I am learning the transform now.

    It will be kind of you guys to share your opinion.

    html, css

    1

    P
    Dave Quah• 670

    @Milleus

    Posted

    Greetings,

    Congratulations on completing your second challenge!

    transform: translate(-50%, -50%); means to move an element to the left by 50% of the current width of the element, and also up by 50% of the current height of the element.

    This is typically paired with position: absolute; top: 50%; left:50%; to center the element horizontally and vertically. For example,

    .parent {
  position: relative;
}
.child {
  position: absolute;
  top: 50%;
  left: 50%;
  transform: translate(-50%, -50%);
}

    You can read more about this under "Is the element of unknown width and height?" at centering css complete guide.

    You can also try adding the position absolute and translate code snippet to your eye image and see it works :)

    I would also suggest looking into using <button> or <a> for interactive elements as it would improve the accessibility of your solution.

    I hope this was helpful for you, happy coding~

    Marked as helpful

    0
  • Dirk• 20

    @Kerraan

    Submitted

    Hi, this is my first challenge here on frontendmentor. Started learning html and css 2 months ago and wanted to practice. This is the first code i share with other people so im curious what you think. I know its a really short (and perhaps easy) challenge but its my first one and a another step in the right direction. Im eager to learn from the community and hear what you all think.

    QR-code component html & css

    1

    P
    Dave Quah• 670

    @Milleus

    Posted

    Hi there 👋,

    Congratulations on completing your first challenge! 🥳👏🎉

    I have some feedback for your consideration:

    1. There are some landmark issues, you can fix it by changing the <div class="center-screen"> to <main class="center-screen">. More about landmarks here.

    2. Always have the alt attribute with <img> elements. It helps to add a text description for screen reader users. If there are decorative images, you can have alt="". More about decorative images here.

    3. Setting font-size: 62.5% on html is... .... a hot controversial topic and non-standard 😅. There's been a lot of debate about this recently on Twitter 🙈. Be careful when using this in team projects because a lot of developers are very used to working with 16px base font size, you may incur a lot of unnecessary wrath.

    Happy coding~ 😊

    0
  • P
    Dave Quah• 670

    @Milleus

    Posted

    Hi there 👋,

    Congratulations on completing this challenge 🥳👏🎉.

    I have a small feedback, and that is when using list-style: none; be careful of the accessibility concerns.

    You can read more about it at this MDN website link and how to fix the issue (shakes fist at Safari ✊).

    Happy coding~

    Marked as helpful

    1
  • P
    Dave Quah• 670

    @Milleus

    Posted

    Hi there 👋,

    Congratulations on completing this challenge 🥳👏🎉.

    I have a couple of suggestions on top of what Abdul has mentioned:

    1. I would strongly recommend to use the right semantic elements, either <a> or <button>, for the "Proceed to Payment" and "Cancel Order" button instead of a <div>. At its current state, a keyboard user would not be able to interact with those elements.

    2. For decorative images, you can leave the alt tag as an empty string. For example, <img src="images/icon-music.svg" alt="">.

    Happy coding~

    Marked as helpful

    1
  • P
    Dave Quah• 670

    @Milleus

    Posted

    Hi there 👋,

    Congratulations on completing your first challenge! 🥳👏🎉

    I have a few suggestions for improvement:

    1. There are accessibility landmark issues highlighted in the accessibility report. This can be fixed by having your body content wrapped within a <main> HTML element. You can find out more about this at the W3C ARIA landmarks website.

    2. A nice way to center content is to add a min-height: 100vh; to the <body> and use either CSS flex or grid. Here is a link on how to center with CSS flex.

    3. SVGs can be imported as an image, there actually isn't a need to copy it into the HTML. For example, <img src="images/icon-star.svg" alt="">.

    4. The background images are missing, see the bg-pattern SVG images in the images folder. These can be added with background-image: url(images/bg-pattern-bottom-desktop.svg) and a mix of other background properties to position the patterns.

    5. There's a typo in the color definition of the .rated-text CSS. Instead of color: --light-grayish-magenta; it should be color: var(--light-grayish-magenta);.

    Happy coding~

    Marked as helpful

    0
  • P
    Dave Quah• 670

    @Milleus

    Posted

    Hi there o/

    Nice work! I like the clever use of Tailwind's group utility class for the image hover 👍

    I do have one minor feedback and that is to use <button> or <a> for the interactive (hover-able/focus-able) elements instead of applying hover directly to the h1, div or p. Using a <button> or <a> element naturally makes the website much more accessible for screen reader / keyboard users because these elements are "tab-able" by default.

    This challenge doesn't explicitly state that whether the interactive elements are links or trigger a pop-up, but it does seem like they are meant to (to me at least). For example,

    • "Hover-able image" could be a button to pop up a bigger image, or a link to a bigger image.
    • "Hover-able Equilibrium #3429" could be a link to a page with more details about this NFT.
    • "Hover-able Jules Wyvern" could be a link to the user's profile page.

    Something for your consideration perhaps. Happy coding! :)

    Marked as helpful

    0
  • P
    Dave Quah• 670

    @Milleus

    Posted

    Tailwind 😍 nicely done!

    I have a couple of minor suggestions:

    • The flex flex-col can be removed as the elements would naturally stack.
    <div class="flex flex-col bg-white p-5 border rounded-3xl w-[24rem]">
// can be updated to
<div class="bg-white p-5 border rounded-3xl w-[24rem]">
    • Tailwind has a space utility that acts like flex/grid's gap, but without having to add the flex/grid property.
    <div class="flex flex-col text-center gap-6 p-7">
// can be updated to
<div class="text-center space-y-6 p-7">

    Of course if the flex flex-col was added with intention, that's perfectly fine as well 😊

    Happy coding o/

    Marked as helpful

    1
  • P
    Dave Quah• 670

    @Milleus

    Posted

    Hi there o/

    I have a few suggestions on improvement:

    • When hover effects are involved, try to use either <a> or <button> elements. Interactive elements typically link you to another page or triggers something to happen on the page. Using the right semantic elements would ensure better accessibility - think of navigating with the keyboard to highlight/trigger these elements.

    • The h2 element by default is a display: block which means it stretches to the full width. As the :hover is added directly to h2, it means that if your mouse is on the empty space to the right of the text, the hover effect would be triggered which is a little weird. Following point 1, I'd suggest something like this to fix the issue:

    <h2><a href="#">Equilibrium #3429</a></h2>

    a {
  color: white;
  text-decoration: none;
}

a:hover,
a:focus {
  color: cyan;
}

    • For image alt text, if it is a decorative image, keep the alt text but leave it as empty, e.g. <img src="/images/icon-clock.svg" alt="">. For non-decorative images, remember to add an alt text describing what the image is.

    • Try to use landmark HTML elements. For example,

    <body>
  <main>
    ...
  </main>
</body>

    Hope this helps!

    Marked as helpful

    1
  • Mahmoud Mohamed Abd Elaziz• 470

    @ehoda9

    Submitted

    I want to know how to make this :

    -bg-pattern-quotation.SVG

    Responsive or how to add it what is the better way

    in background or just add it position absolute <img>

    Grid

    1

    P
    Dave Quah• 670

    @Milleus

    Posted

    hi there o/,

    I think using background-image to add the quotation image is fine. A small change that could help is to define the background-position to start from the top and right with an offset so that scales more consistently with the design.

    e.g.

    background-position: top right 15%;
/* or you can also do this */
/* background-position-x: right 15%; */
/* background-position-y: top; */

    Adding the quotation image with <img> element and position absolute works too, but it may require a bit more work. Most likely the text will require adding a z-index and a position: relative (for z-index to work) to place it above the <img> element.

    If using <img> element with position absolute, I would also define the position to start from the top and right with an offset, e.g. top: 0; right: 15%;.

    Hope this helps :)

    Marked as helpful

    2
  • P
    Dave Quah• 670

    @Milleus

    Posted

    Hi there o/,

    To get the z-index to work where the text is in front of the quotation marks image, you'll need to set a z-index on the text (in this case the <h4> element) and also set the position property to anything other than static which is the default value. For example,

    .cards.Daniel h4 {
  position: relative;
  z-index: 1;
}

    An alternative solution to this which might be cleaner is to add the quotation marks through the background-image property instead of using an <img> element. It will naturally be behind the text, and you can adjust the position with the background-position property. For example,

    .cards.Daniel {
  background-image: url(/images/bg-pattern-quotation.svg);
  background-repeat: no-repeat;
  background-position: top right 15%;
}

    Here are the MDN links to find out more:

    Hope this helps

    Marked as helpful

    0