Skip to content
OverviewSolutions
14
Comments
17
Marcus Rangel
@WebDevCampos

All comments

  • Marcus Rangel•460
    @WebDevCamposPosted about 2 years ago

    Hi there! Great work! I've noticed that the buttons don't have hover states like the challenge asks. Besides that it's awesome!

    0
  • Marcus Rangel•460
    @WebDevCamposPosted about 2 years ago

    Hi there! You did great! I just have a few observations to make if you allow me. Firstly, you need to wrap How did we do in a h1 tag, so the accessibility report won't find any errors. You can adjust it's font size and weight via CSS. Secondly, you could apply an addEventListener to the form itself, and iterate through all radio inputs, and apply a .forEach() method, for example. Your solution has some of these suggestions implemented, but what I mean is, you can have even less code, because there are a few things that aren't needed. If you fell like, you can take a peek at my solution : ) Please , feel free to reach me out and maybe we can work together on something! Kind regards!

    1
  • Marcus Rangel•460
    @WebDevCamposPosted about 2 years ago

    Hi there! If I may, there's an observation I would like to make. Firstly, the component needs adjustment in it's responsivity, because it's stretching a lot haha. In my phone, for instance, it's width barely reach 200px. Secondly, you could wrap your index.html code in a main tag, for the sake of accessibility. And finally, the link provided for your code solution leads to the qr component solution, not this. I hope I could be helpful. Please, fell free to reach me out! Kind regards!

    Marked as helpful
    0
  • Marcus Rangel•460
    @WebDevCamposPosted about 2 years ago

    Hi there! You did good, and if you allow me, I have some suggestions to make. Firstly, wrap all your index.html code, in a main tag. Secondly, you do not have to create a function specifically for one button at a time, you can select them all through document.querySelectorAll() and apply .forEach() method, for example. You can take a peek at my solution : ) These are the first steps. Please fell free to reach me out and we can work together! Kind regards!

    Marked as helpful
    1
  • Marcus Rangel•460
    @WebDevCamposPosted about 2 years ago

    Nicely done man! If you allow me, I have a few observations, though. First, wrap all your code in a main tag. Second, the image should have a text inside the alt attribute, since it's the component's core. Great work, keep coding! If I could help let me know, and if you have any questions or doubts, let's work together! Kind regards!

    Marked as helpful
    1
  • Marcus Rangel•460
    @WebDevCamposPosted about 2 years ago

    Nice job man! If you allow me, I have some suggestions to make: First, the img tag must have an alt attribute, mainly in this case, for the image is the product. Second, a small adjustment in border-radius, so it gets closer to style-guide parameters , and finally wrap your code in a main tag. If I could help, let me know! Kind regards!

    Marked as helpful
    1
  • lieneil•560
    @NJVSSubmitted about 2 years ago

    I really enjoyed doing this challenge.

    I use Javascript Promise for form validation. At first, I planned to write a simple function that returns true if there are no invalid inputs, but the problem is that after running this function, I'll need to add another if-statement inside the submit event to check if the function returns true. And I think that's a redundant code.

    With Promise, I could just resolve it and pass the form data, or reject it and pass the invalid inputs and prevent the form from submitting. Tho this project prevents the default form submit even if there's an invalid input. I had intended to create reusable form validation, and I believe this is an excellent example of why using Promise is better than my initial solution.

    I'm not sure if I did everything correctly, so if you have any suggestions, don't go easy on me, It really helps. Thaaaanks <3

    Interactive card details form

    1
    Marcus Rangel•460
    @WebDevCamposPosted about 2 years ago

    Hi there! You have an amazing work! This very project was the most challenging for me so far and I noticed you have some issues that I've faced myself. See, in android devices the maxlength attribute doesn't work like in PCs. So I've found a pen that fixed that, by K-ro. Check it out and let me know if it helped you! Kind regards!! https://codepen.io/K-ro/pen/NPWBNO

    Marked as helpful
    2
  • Marcus Rangel•460
    @WebDevCamposPosted about 2 years ago

    Hi there! Congratulations on posting your first challenge! You did great, but if you allow me, here are a few suggestions: Firstly, always look at style-guide.md file that comes with the project. There you'll see the proper colors and sizes, for typography and for the elements. So you can see that you have to fix the Improve your front-end skills by building projects color. Actually it leads me to another topic: all projects must have a h1 tag, so this very text is a h1. If you feel that the default font-size for this element is bigger than you like, than just set your own size. Here is a tip to center your main element: body { background-color: hsl(212, 45%, 89%); font-family: Outfit; display: flex; justify-content: center; align-items: center; height: 100vh; } Doing this, you won't need to set a margin that big for your component and you won't need to set a heightor overflow either. These are the frist steps to improve your solution. If you feel like, you can reach me out and we can work together! Kind regards!

    Marked as helpful
    0
  • Marcus Rangel•460
    @WebDevCamposPosted about 2 years ago

    Hello there! You did great! For this project I have put lots of divs, but you could handle it with less code than me. Although, if you allow me, I have a few observations. First, when in mobile, the card has something that is causing it not to proper fit, some kind of overflow. I get this a lot when using padding or margin where is not needed, maybe you could review it. Secondly, there is sort of a horizontal rule between the image and the text, and at last, the word Companies has a typo, it happens a lot haha 😅. So, my advice is to use fixed widths in this project. You can set a max-width:1440px to the card itself, when in desktop and max-width:375px when mobile. I didn't check the code in deep, but I think you're fine, and have just these few adjustments. If you feel like, please reach out! Kind regards!

    0
  • Gwen•40
    @gwencodingSubmitted about 2 years ago

    My questions for the community as a newbie on this challenge :

    I had trouble making the site responsive so someone could look at it and help me improve this part. In phone mode the image is smaller than the example shown and the main text is not aligned the same way.

    Is there a particular order when coding? Is my code lenghty?

    Other feedback welcome ! 🙏

    Responsive stats preview card

    1
    Marcus Rangel•460
    @WebDevCamposPosted about 2 years ago

    Hello there! Yes, this is a tricky one challenge. If you allow me, here are a few suggetsions: For the image troubble, you may look for picture tag in HTML 5.

    There are other issues regarding responsiveness. First of all, you would want to set the following styles in your body tag:

    body {
    background: var(--main-background);
    display: flex;
    flex-flow: column;
    justify-content: start;
    align-content: center;
    min-height: 100vh;
}

    It will allow you to set the main tag a justify-content:space-between, and the two sections of the card will be equally apart.

    For the main tag it self, you can set

    main {
    display: flex;
    justify-content: space-between;
    max-width: 820px;
    background: var(--card-background);    
    margin: 7rem auto;
}

    The max-width attribute will avoid the stretch effect. For .main-content and .img-background, you'd want to set a width:100%, for the .main-content a padding:2rem is enough and finally for the order of element's to swap, you may want to set

    @media screen and (max-width:820px){
main {
    display: flex;
    flex-direction: column;
      width: 375px;    
}
.main-content {
    padding: 2.5rem;
    text-align: center;
    order: 2;
}
}

    If you have further questions, please, reach me out, I'll be more than happy to contribute somehow. I must say that looking at your solution now, mine might be over elaborated, I'll review my code right now haha 😅.

    Marked as helpful
    0
  • Mohammed El Fraiji•160
    @momoel00Submitted about 2 years ago

    It took me about half an hour to do it, I need your opinion on the design and the code . If it is a simple modification, it will help me a lot, thank you in advance.

    QR Code Challenge

    2
    Marcus Rangel•460
    @WebDevCamposPosted about 2 years ago

    Hi there! You did well, but if I may, there are a few corrections to make. Firstly, the background-image is repeating, you can fix that setting css background-repeat:no-repeat. Secondly, please review the component's width and height. You don't have to set a relative unit such as percentage, you can set a fixed size. Read the style-guide.md file and you'll see all the requisites for the project. The size for mobile devices for instance, it's 375px and for desktops it's 1440px, maximum. All projects must have an h1 tag, so you can replace the h3 and set the font-size till it gets close to final result. You can check the style-guide.md file to set the proper colors to the elements as well. Any questions or further assist, don't hesitate to reach me out! Kind regards!

    0
  • Marcus Rangel•460
    @WebDevCamposPosted about 2 years ago

    Hello there! Great work, smooth transition on icon menu! I have noticed an issue, however. If you open the side menu when screen is small, when you increase size screen again, the darken area remains. I guess that's the only thing I noticed, but you did great working with both grid and flexbox. Kind regards!

    Marked as helpful
    1