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

All comments

  • P
    j5 2,290

    @jmnyarega

    Posted

    • To answer your question, there shouldn't a space between the selector and the pseudo-class :hover i.e .download-primary:hover.
    .download-primary:hover{
        background-color: var(--secondary-blue);
    }
    

    Good work on the challenge 👏

    Marked as helpful

    1
  • P
    j5 2,290

    @jmnyarega

    Posted

    Nicely done, I have one suggestion:

    • Look into relative units(em/rem), this will enable users to adjust the font sizes of your app from their browser settings.
    0
  • P
    j5 2,290

    @jmnyarega

    Posted

    Nice work on the challenge, I have a few recommendations:

    • You don't have to duplicate navigation HTML code for mobile and desktop. Use media queries to alter the styling depending on the screen size.

    • section should be under main not header.

    • Instead of leaving the alt attribute of the logo image undefined, you can pass the name of the company. In this case, Project Tracking will suffice.

    • Add either role="presentation" or alt="" aria-hidden="true" to a decorative image so that bots can skip it. without alt attribute, the screen readers will read out the path.

    • Add a hover state to the button and links.

    Otherwise, nicely done 👏👏

    Marked as helpful

    0
  • P
    j5 2,290

    @jmnyarega

    Posted

    Hi @designmonkey714,

    • You have opacity: 0.9 on cover-overlay, it should work fine without the opacity.
    • Look into relative units(rem/em/vh/vw), they enable users to adjust e.g font sizes from their browser settings.

    Otherwise, great job on the challenge.

    1
  • P
    j5 2,290

    @jmnyarega

    Posted

    Hi @Deor1496,

    Congratulations on getting this done.

    • This class w-2/3 is breaking the responsiveness on small screens. I can see the class translates to width: 66.66% which is not wide enough for mobile view. You can increase that to width: 90% => w-9/10.

    • On the background image, set background-size: 100vw 40h.

         background-image: url(./images/pattern-background-desktop.svg);
         min-height: 100vh;
         background-position: top center; // you dont need to set this
         background-repeat: no-repeat;
         background-size: 100vw 40vh;
      
    • Avoid setting solid heights, use min-height instead of height, i.e min-height: 100vh

    • For such a small project, I will recommend not using libraries. In fact, it can cause a lot of confusion.

    Hope this helps, happing coding!

    2
  • P
    j5 2,290

    @jmnyarega

    Posted

    Hi @Victorrrocha.

    You did great on the challenge, and I can't seem to break the responsiveness 🙂. The page responds well to all screen sizes. Good job on that, I have a few suggestions:

    • Look into relative units in fact, I will encourage you to use them almost everywhere. This will allow users to change font sizes from their browser settings.

    • Add hover state on the buttons.

    Marked as helpful

    0
  • @atalle

    Submitted

    Hello community!

    I struggled with implementing the BG pattern and am not satisfied with how the pattern transitions between breakpoints. Would love feedback on how to improve this, as well as general feedback on the structure of my HTML & CSS classes. Thank you!

    P
    j5 2,290

    @jmnyarega

    Posted

    You can use the background CSS property to place the images on the corners, but first:

    • Center your card property, I will suggest having min-height & container width's properties be set on the body:
    • Use background properties to position images.
    • play around with background-size property to place the images in their correct positions.
      body {
        min-height: 100vh;
        max-width: 90em;
        width: 90%; 
      
       background-image: url(.../top-left.png) url(bottom-right.png);
       background-repeat: no-repeat;
       background-position: <some-value some-value>,  <some-value> <some-value>
      }
      
    
    You can check out CSS background property on [MDN](https://developer.mozilla.org/en-US/docs/Web/CSS/background) for more
    
    Happy coding!
    

    Marked as helpful

    0
  • P
    j5 2,290

    @jmnyarega

    Posted

    You did an impressive job on this one, I only have one suggestion:

    Marked as helpful

    0
  • @stevenabaco

    Submitted

    Hello fellow Developers,

    This is my first challenge with Frontend Mentor. I singed up to practice my coding skills on real projects and have something to prove I've got the skills. Also to help out other developers on the same learning track.

    I built this component using plain HTML and CSS. Built with Mobile First design process. Only added one @media query point at 768px.

    If anyone has time to review and let me know what they think and any recommendations on improvement I would greatly appreciate it. I'm always interested in ways to improve and welcome any feedback from the community.

    Thanks and looking forward to your feedback and comments!

    P
    j5 2,290

    @jmnyarega

    Posted

    Hi @stevenabaco you did very well on the challenge 👏👏. I have a few recommendations for you:

    • Instead of background-size: initial, you can have something like background-size: 100vw 40vh`.
    • Look into relative units, using rem/ems enables users to resize fonts and sizes from their browsers.

    Happy coding!

    Marked as helpful

    1
  • P
    j5 2,290

    @jmnyarega

    Posted

    Hi @Albusflames, good job on completing the challenge. I have a few suggestions for you:

    • Everything with a main tag.
    • Looks like the font family & font colours are a little bit off, you can refer to style-guide.md for the right fonts.
    • Look into relative units, this will allow users to resize font sizes from their browser settings.

    Otherwise, this is a good start.

    0
  • P
    j5 2,290

    @jmnyarega

    Posted

    Hi @AnuRobo, good job on the challenge. I have a few suggestions:

    • Avoid setting solid heights, instead of height: 100vh use min-height: 100vh.
    • To remove the horizontal scrolling, add overlow-x: hidden on the body.
    • Prefer relative units to px, this allows users to adjust font sizes from their browser settings.
    • Wrap your card with the HTML 5 tag main.

    Happing coding 🙂

    Marked as helpful

    1
  • P
    j5 2,290

    @jmnyarega

    Posted

    Impressive work on the challenge @JustAFatRaccoon 👏👏

    I have a few recommendations:

    • Prefer relative units to pixels.
    • Add hove state on the button.
    • You are switching to desktop layout too late (1440px), I suggest you switch at around 500px.

    Marked as helpful

    0
  • @SKLymer

    Submitted

    I was wondering if there was a more dynamic solution to set the width and height of the hero segment of this order card. I ended up using static units(px) to get the proper height. But somehow it felt clumsy and unprofessional, trying to gauge it against the design image.

    P
    j5 2,290

    @jmnyarega

    Posted

    Good work on the challenge @SKLymer, here are my suggestions:

    • Use HTML 5 tags, instead of div.main-cont & div.order-card use main tag and section.order-card` respectively.

    • You can get rid of the horizontal scrolling by adding overflow-x: hidden on the body.

    • Instead of having two img and switching back and forth between screen sizes, HTML provides an srcset attribute with can handle without CSS. It's pretty amazing, check it out.

    Happy coding 🙂

    Marked as helpful

    0
  • P
    j5 2,290

    @jmnyarega

    Posted

    Good job on the challenge @sergii-moroz 👏👏👏👏

    I have a few recommendations:

    • Add focus state on the theme switcher element, I can not change the theme with my keyboard. This can be frustrating for keyboard users.
    • Change cursor to pointer cursor: pointer when hovering on the theme switcher element.

    Otherwise, good work.

    Marked as helpful

    0
  • Aituos 40

    @Aituos

    Submitted

    Any feedback is appreciated! I have one specific question though: If I set 'aria-hidden="true"' on a decorative image for example, to hide it from screen readers, can I completely skip adding the alt text? Or should I set it to an empty string (alt="")?

    P
    j5 2,290

    @jmnyarega

    Posted

    Hi @Aituos, good job on the challenge.

    • Yes, if you set aria-hidden=true you need to have the alt="" too.

    • You can change div.main-section into section tag.

    • A nitpick, Summary text on the title should be capitalized.

    Otherwise, good job on the challenge. Keep coding 🙂

    Marked as helpful

    1
  • P
    j5 2,290

    @jmnyarega

    Posted

    Amazing job on the challenge @ashiqfury, I only have one suggestion:

    • Add max-width on the accordions.
    0
  • tomas938 385

    @tomas938

    Submitted

    Another challenge 🎈🎈🎈 feedback is welcome i have no idea how to do pixel perfect design i tried chrome extension but i need definitly improve

    P
    j5 2,290

    @jmnyarega

    Posted

    Hi, my name is Josiah. I have a few suggestions for you:

    • main should not be a descendent of section. Make it a descendant of body instead. I suggest the following layout.
      <body>
         <main>
              <nav>
                // header here...
               </nav>
               <section>
                 // image & content here....
               </section>
         </main>
      </body>
      
    
    - Add a focus state to the buttons and links, this will allow keyboard users to use your site. 
    
    - You can use `align-items: center` to align content inside `main`  instead of using `padding`.
    
    Otherwise, good job on the challenge 👏👏
    

    Marked as helpful

    1
  • P
    j5 2,290

    @jmnyarega

    Posted

    Nice work on the challenge @KaliRodri. A few suggestions here:

    • Annual plan should be in a strong tag, not h4
    • You can use viewport units(vh, vw) to set background-size of the background pattern.
         background-size: 100vw 40vh;
      
    • Prefer relative units(rem/em), check out this resource em-vs-rem-vs-px

    Keep coding 🙂

    Marked as helpful

    1
  • P
    j5 2,290

    @jmnyarega

    Posted

    Great work on the challenge @Adekola, my recommendations:

    • At around 863px(window width), the images on the gallery component are stretched out to fit their containers, this happens immediately after desktop to mobile transition. To prevent this behaviour, you can use object-fit: cover on the images.

    • You can use Grid or flex layout on the image gallery. You won't need a clear fix. These layouts were created to fix such hacks. 🙂

    • Semantically, client testimonials should be a list of items.

    0
  • P
    j5 2,290

    @jmnyarega

    Posted

    Looks good @GodlyCodex. Good work.

    1
  • P
    j5 2,290

    @jmnyarega

    Posted

    Impressive work on the challenge @shake88junt, the page responds well to mobile & desktop 👏👏👏👏.

    A few recommendations:

    • I can't navigate the site using the keyboard only, you may want to add a focus state on buttons and links.
    • Prefer using relative units
    • Instead of <div class="main"> you can use HTML5 tags <main>.

    Otherwise, great work on the challenge.

    0
  • P
    j5 2,290

    @jmnyarega

    Posted

    Hi @ddaniel27.

    • You are on the right path, you can use grid-template-areas to make the solution more elegant. On the design, we have 4 cards, supervisor, team-builder, 'calculator' and karma. Grid areas allow us to create layouts visually in our code, check this out.
    display: grid;
    grid-gap: 1rem; // some spacing between cards
    grid-template-areas:
         'supervisor   team-builder   calculator'
         'supervisor    karma             calculator';
    }
    

    Now centre supervisor and calculator vertically:

    .supervisor, .calculator { align-self: center; }

    And that's it. 🙂

    Marked as helpful

    1
  • P
    j5 2,290

    @jmnyarega

    Posted

    Nice one on the challenge, I have a few observations

    • There is a small gap between the image & the bottom border of its container, you can fix that by add display: block to the image.

    • buttons and links do not inherit fonts & colors by default. I think that's why the styles aren't applying to the buttons at bottom of the card.

    • Since buttons are focusable by default, you don't need to have links inside buttons, something like <button> Proceed to Payment </button> will be just fine. This will also reduce the styling nightmare 🙂

    • Prefer using relatives units.

    Happy coding 🙂

    Marked as helpful

    1
  • P
    j5 2,290

    @jmnyarega

    Posted

    Hi @EdgarBadillo, great work on the challenge. To answer your question, you can use javascript to track the open item.

    Marked as helpful

    0