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

  • @dknyd

    Submitted

    Hey All,

    I have finished this challenge, although for the life of me I could not get that damn exclamation mark icon where it should be, so within the email input field. -which I know is pretty stupid-, so if anyone has an advice for placing it where it should be, I would be grateful. :)

    Thanks as usual!

    Daniel

    P

    @juanpb96

    Posted

    Hi Daniel, good job on your solution!

    I was looking at you code and I see you can do some fixes. You should consider the use of position: relative; in your #inputContainer or changing the layout of your input just a bit. I think you can use #inputContainer with display: flex; and place the button and icon better. The border could be moved to the container, so you can adjust styles in an easy way.

    Hope it helps!

    Marked as helpful

    0
  • @mehra-sourav

    Submitted

    • Should I explicitly define width in .card class or should it be handled by media query (the commented-out code in the end)? What's the best practice?
    1. Am I using too many divs (for header-text and description-text) when I could have used h1 and p for the texts? Or is it just a personal preference?
    P

    @juanpb96

    Posted

    Hey Sourav 👋

    Good job on this one!

    I think I can help you with your questions and include additional comments:

    • It is good to define width in your .class and you should change it according to what you want with media queries. I recommend starting with a mobile-first approach.
    • You can use different tags to provide a more sematic HTML structure in your code. For example, your <div class="attribution"> should be <footer class="attribution"> or <div class="card"> could be <main class="card">.

    I think it is not a personal preference in general. If you use tags properly, your site increases in accessibility and best practices. You can take a look at HTML reference as it has helped me when I wonder about tags.

    Hope this can be useful for you 😃

    Marked as helpful

    0
  • P

    @juanpb96

    Posted

    Hi Harsh 👋

    Your solution looks great! 👍

    I think you can consider some points to improve your project:

    • You can improve you HTML structure by including more semantic tags. For example, your <div class="main"> could be <main>.
    • The background is disappearing in desktop screens, have you added the correct image path?
    • Card spacing is too small, maybe you can check your styles and add some padding 🤔
    • Try to use CSS pseudo elements (::before or ::after) to avoid unnecessary HTML elements such as <div class="bg_image">

    Hope this can help 😃

    0
  • Kim 340

    @Mismisty

    Submitted

    I would love any and all feedback on this challenge, from the overall look to the code itself. Please let me know if there is any code that is unnecessary or could have been shorten somehow. Thanks, and have a great week!

    P

    @juanpb96

    Posted

    Hey Kim! 👋

    Your solution looks good! 🥳

    I have some tips for you to improve your project:

    • I think you can change your div class="attribution" to footer class="attribution" this way you add a more semantic tag in your HTML 😉
    • I'm not sure why you used display: flex in your body but this is causing two columns, so the layout looks a bit weird.
    • Stars in .review are using inline styles, I consider you can include a class or play with selectors to easily add the margin that you need. Maybe something like .review > img { margin-right: 9px } could work.

    Hope this helps! 😃

    Marked as helpful

    0
  • P

    @juanpb96

    Posted

    Hi Marina! 👋

    Congrats on your solution! 🙌

    I've seen some things that you can do to improve your project:

    • In your 'Proceed to Payment' button: You could increase its font-weight and make it use the same font-family as it is taking the default from browser styles. Also, include cursor: pointer so the user will know that this button is clickable.
    • Try not to use <br> and play with margin instead 😋
    • Adding hover states for links and buttons could be awesome 😎

    Hope you find this useful!

    Marked as helpful

    1
  • P

    @juanpb96

    Posted

    Hi Staniuu! 👋

    Good job in this one! 😃 I was looking at your solution vs the design and I found out few things you can do to improve your project:

    • You could add a gray background color
    • Are you using the correct font for your card titles? And don't forget to make them uppercase 😉
    • Try including border-radius in the first and third card
    • Learn more buttons could have more padding on the left and right sides

    Hope this helps!

    Marked as helpful

    1
  • Kapline 140

    @TrueKapline

    Submitted

    I am really unsure about the modernity of my solution. Many people are using rems instead of pixels, and I don't know if I'm also should use them at this project.

    P

    @juanpb96

    Posted

    Hey Kapline 👋

    Your solution looks great! 😎... I've read this article Pixels vs Rem. It explains very clearly the difference and when to use each one. Hope you find it useful to solve your doubts!

    1
  • P

    @juanpb96

    Posted

    Hi Carlos 👋

    Good job on this one! 💪. I noticed that you implemented all the info in your icons as a link, that is what it looks like when you hover over them 🤔. Try to remove hover styles when there is no data available and apply opacity to your text as you did with the icon. Another thing I saw is that links for user blog, twitter and company aren't working as expected.

    0
  • P

    @juanpb96

    Posted

    Hi Nigel 👋

    Your solution is great! I've noticed that you are positioning your button with percentages and that causes a weird behaviour when someone sees your page in a large screen. I would recommend moving your button inside .form-box and using display: inline-block in your .button (I wonder why you added that additional div 🤔). Then, you can try fixing sizes and position values to make it look as the design.

    0
  • Jasper 40

    @Jasper-Ik

    Submitted

    I had in mind for the layout to use display flex to center the container but the footer portion just had to make it a two column layout so i opted for using margin auto

    P

    @juanpb96

    Posted

    Hi Jasper 👋

    Good job on your solution! 👍... As you want to use flex to center your content I will give you one idea for you to try.

    In your body set width to 100% and height to 100vh, this takes the total size of the screen (set margin to 0, just to prevent horizontal scrolling). Then, add display: flex and flex-flow: column to keep the layout that you want. To centralize your content use justify-content: center and align-items: center, this will do the magic 🧙. Finally, remove margin in your .container class.

    Another thing I saw is that you have 'Document' as your page title.

    0
  • P

    @juanpb96

    Posted

    Hi @andrytoni 👋 Good job in your solution!

    If you want to improve your project by centering your div differently, I would recommend checking this example: How to center a div with transform

    0
  • P

    @juanpb96

    Posted

    Hi Gabriel 👋.

    Good job on your solution! I think you can solve your problem centralizing your content differently. Have you tried grid? ... If you want to, add a new parent container for your main content (or using your body tag works too). This new element should take 100% width and 100vh heigth. Lastly, add place-items: center and that should do the work. Don't forget to remove those margins to avoid horizontal scrolling 😉

    Marked as helpful

    1
  • @UnTalPeluca

    Submitted

    How can I apply color to the image? In "Order summary component" I used background-image and background-color, but in this case it didn't work

    P

    @juanpb96

    Posted

    Hi 👋

    Good job on your solution! 👌

    If you want to apply color to your image, try this: background-blend-mode in your .card-image class.

    On the other hand, I think you should remove the media query @media screen and (min-width:375px) as you want to work mobile-first.

    Happy coding! 😄

    Marked as helpful

    1
  • P

    @juanpb96

    Posted

    Hi Tomiwa.

    Good job on you solution 🙌.

    I have a suggestion for you:

    • You can add a span to your anchor tag for social media icons. This is a way to put text there and give screen readers the possibility to know the meaning of the icon and increase your accessibility score.
    • If you want to do what I suggested above, then you will need to implement a class that hides that text from the design, I recommend this:
    .sr-only {
        width: 1px;
        height: 1px;
        position: absolute;
        margin: -1px;
        padding: 0;
        overflow: hidden;
    }
    

    Hope that helps. Happy coding! 🥳

    Marked as helpful

    1
  • @Sloth247

    Submitted

    === Re-done on 2 September 2021. ===

    • Added proper email custom validation in JS.
    • Improved accessibility for error messages.

    === This is original issues raised in July 2021 ===

    This is my first submission on frontendmentor. I could not solve below, however most of design is satisfactory;

    • Hover on arrow button: hover does not work on buttons?
    • Background image: do I need repeat and gradient? I could not figure out to do the same as original design.
    • addEventListener('submit', function (){} : it did not work, so I used click instead to show error messages and icon.

    I appreciate if you provide me good feedbacks.

    Many thanks

    P

    @juanpb96

    Posted

    Hi @Sloth247

    Good job on your solution 👍

    To help you with your questions I consider:

    • Hover works for buttons but you are having a problem with a background property that is not working, try using opacity and you will see the change on hover. So, please review your background property 👁️‍🗨️
    • You can use backgroud-repeat: no-repeat. However, I can't understand your question pretty well.
    • Did you use form.addEventListener('submit', function(){})?

    Hope my suggestions could help you. 🙌

    Happy coding!

    Marked as helpful

    2
  • @rogerdummer

    Submitted

    Hi, please feel free to give me your considerations and feedbacks. It will be very useful to my learning process.

    P

    @juanpb96

    Posted

    Hello Roger Dummer!

    Great job on your challenge 🙌, I have some suggestion for you:

    • Review your media queries, in my opinion, you can use dev tools to look what could be a better breakpoint to start changing your layout (I consider 1100 or 1200 px).
    • Play with the font-size through media queries breakpoints.
    • Your container is maybe too wide, you can include the max-width property to solve it. This could help if someone is watching your page on a tablet.

    Everything else looks good!

    Happy coding 😄

    0
  • P

    @juanpb96

    Posted

    Good job in your solution!!!

    It looks great for every view.

    Happy coding 😁

    0
  • P

    @juanpb96

    Posted

    Hey @eh-ins , good job on your solution!

    I have some suggestions for you:

    • You could try use height in your body tag and give it a value of 50rem for example, to give you your card a posibility to be more centered in the page.
    • When the size of the window is about 900px and you start reducing more this size, the content looks too compacted and it is difficult to read. In my opinion, you could include a media query in this point.

    Everything else seems great.

    Happy coding!

    0
  • P

    @juanpb96

    Posted

    Hey I have looked at your solution and I think you need to improve different things.

    In my personal opinion you can try:

    • Use one class name for those three divs inside .div0 as all of them shared css properties.
    • Reduce the buttons size to be more user friendly. Decrease font-size could help.

    Your mobile version looks good!

    1