Skip to content
OverviewSolutions
15
Comments
17
Adrian H
@AdrianX19

All comments

  • Adrian H•420
    @AdrianX19Posted over 2 years ago

    Hi @jesuisbienbien!

    congrats on completing this challenge! really good job! :) The issue you have is caused because <small> elements is an inline and for such elements the top and bottom margin/padding has just not effect, you can do a couple of things to make it working for example you can replace <small> with some block element like <p>, you can change your current element to a block element by adding 'display: block' or you can add margin to your container 'email-error-container' :)

    Marked as helpful
    1
  • Adrian H•420
    @AdrianX19Posted over 2 years ago

    Hi @jamespaulmiranda!

    Welcome on frontendmentor and congrats for submitting your first challenge! :) a few suggestions:

    • to get rid off the accessibility/html issues there are two things you should do, first use semantic tags, so instead of having 'div' for your container use 'main', and instead of having 'div' for your footer, well.. you can use 'footer' tag! :). Second thing is the h3 you used, heading levels shouldn't be used for the styling purpose, it's just not a good practice and we have CSS for that purpose! so changing h3 to h1 and changing the fontsize through CSS would do the trick
    • you might consider giving some min-width to your card so it doesn't get squeezed so much with smaller viewport width
    • you seem to be lacking the mobile version, you can achieve it on many ways but using media queries it's a good call most of the times! (https://www.w3schools.com/css/css_rwd_mediaqueries.asp) :)

    Have a good day! :)

    Marked as helpful
    0
  • Adrian H•420
    @AdrianX19Posted over 2 years ago

    Hi @MisterSlimak!

    really good job! If you'd like to get rid off those accessibility issues just replace your container 'div' with 'main' tag :)

    0
  • Adrian H•420
    @AdrianX19Posted over 2 years ago

    Hi @bigmac369!

    Congrats on submitting the challenge! A few minor things I've noticed:

    • the accessibility issues you have can be solved by changing the #root 'div' to a 'main' tag
    • background colour of the join community section is a bit off comparing to the design, it should be white
    • monthly subscription text should be aligned to left, to achieve that you should probably set 'align-items' property to 'flex-start' for this section
    • the 'monthly subscription' and 'why us' cards should be aligned, I believe you could achieve that by changing justify-content for those cards to 'space-between' and them add the same top and bottom padding for both of them

    Keep it up! :)

    Marked as helpful
    1
  • Renan Tiscoski•1,240
    @DonUggioniSubmitted over 2 years ago

    Howdy!

    It's a simple project using CSS grid. Could have used a few more custom classes for the headings and saved a few lines of code in the media queries but since it's such a small project, I didn't bother.

    The breakpoint for mobile view is below 500px so in order to check it, you might need to use the dev tools to reach those numbers.

    As always, any feedback is welcome!

    Simple price component using CSS Grid

    1
    Adrian H•420
    @AdrianX19Posted over 2 years ago

    Hey @DonUggioni!

    A hover effect for the button would be a nice cherry on top but other than that really nice submit!

    0
  • Adrian H•420
    @AdrianX19Posted almost 3 years ago

    Hi @DarrickFauvel

    really good job, well done! I would just try to make the sizes a bit closer to the design, right now the card seem to be pretty narrow, I would suggest increasing the max-width on .card element and I would increase padding for the .card-body, other than that it looks really good! :)

    Marked as helpful
    1
  • Adrian H•420
    @AdrianX19Posted almost 3 years ago

    Hi @MishalZia!

    Good job on submitting the challenge! :) Just a couple of small things I've noticed

    • the whole layout looks nice but the elements seem to be a bit too small, I would play around a bit with it, alter some widths/heights and font sizes so it more reflects the designs (there is nice chrome extension called 'Pixel Perfect Pro' which will help you to keep your sizes close enough if you don't have access to figmas)
    • font colors and font weights seem to be a bit off especially for reviews section, I think customers names should be bold and the opinions itself should be white
    • seems like you completely missed the mobile version, if you're not sure where to start this might come in useful: media queries
    1
  • Adrian H•420
    @AdrianX19Posted almost 3 years ago

    Hi @PitagoraStrike!

    congrats on completing the challenge, really nice job! I've got just some small suggestions:

    • your background colour is a bit off comparing to the designs - although I bet you've noticed already ;)
    • the width of review cards is a bit too small plus the alignment of the first review and the third review is a bit off (the first card should start where the main text starts and the third card should end where the last stars container ends) - you can easily fix this by changing your justify-content for card containers to space-between
    • regarding your accessibility issues, I see in your code that you are using <h4> which seems like you used it just to get smaller font size - heading shouldn't be used for styling, changing it to <h2> and styling in css would be the right approach although in this case I would just use <p>. About the other issues changing divs to semantic tags like <main> and <section> should do or you can just add 'role' attributes to existing divs

    Hope you will find the feedback helpful! Happy coding! :)

    Marked as helpful
    0
  • Adrian H•420
    @AdrianX19Posted almost 3 years ago

    Hi @rolando-17

    Great to see some creativity over here! :)) Congrats on submitting the challenge! Just a few suggestions:

    • try to use semantic tags when it's possible, in your case you could for example change the container <div> to <main>
    • don't use headings for styling, always start with <h1> and if you need to change how it looks that's where you can use your CSS powers! also not everything should be a heading - there is also <p> tag that can come useful
    • your dividing line is too thick I would say and I would suggest using border in this case instead of a <hr> element
    • the card background pattern as well as dividing line are sticking out from outside the card a bit, try to play around with width and maybe try using some relative values instead of absolute ones
    • you're missing pattern in the main background and the font family is not the one required by design

    Hope the feedback helps you a bit! Have a good day

    Marked as helpful
    1
  • Adrian H•420
    @AdrianX19Posted almost 3 years ago

    Hi @cyberspatial!

    The other way to bite this patterns in the background would be to use background-image to the body (it accepts multiple arguments) and then you can use background-position to place the circles wherever you want :) also I would suggest to use <main> tag for your card container instead of a <div>.

    All in all really good job! :)

    Marked as helpful
    2
  • drunken_neoguri•290
    @DrunkenNeoguriSubmitted almost 3 years ago

    Hello!

    This project is my 4th challenge! XD

    The previous project was also difficult, but in this project, it was difficult to place the background image and specify the 'position' of the profile picture (avatar) area.

    I've completed the challenge, but looking at the code I wrote, I'm not sure it's good code.

    Now, after about a month of studying, please give me a lot of advice and tips! Reply is always welcome. thank you!

    4th Challenge: profile card

    2
    Adrian H•420
    @AdrianX19Posted almost 3 years ago

    Hi @DrunkenNeoguri,

    Good job! if you'd like to get rid of this accessibility issue you can change <div class="namespace"> element to <h1> - keep in mind that if you do so you will have to adjust some styles as well!

    Marked as helpful
    1
  • Adrian H•420
    @AdrianX19Posted almost 3 years ago

    Hi @ahmed9aks,

    Congrats on completing the challenge, nice job! :) I've got a couple of suggestions, maybe you'll find them useful

    • to get rid of your accessibility issues just replace your container <div> with <main> - it's good to get familiar with semantic tags!
    • I see you are using h2 for the name, html tag shouldn't be used to style elements so I would suggest to use h1 and style it to your needs
    • for your footer try to use justify-content: space-around, that will give you result closer to the designs
    • you've got small typo in your CSS inside the media quries: padding: 145px, 25px; the values should not be separated with the comma
    Marked as helpful
    0