Skip to content
OverviewSolutions
11
Comments
28

Saulius K.

@TH3RIVUK560 points

An exciting up and coming developer, still very early in the journey to conquering the internet. =)

I’m currently learning...

HTML | CSS | JS

Latest solutions

View more solutions

Latest comments

  • Saulius K.•560
    @TH3RIVPosted over 2 years ago

    Hi, @stagnant-sys!

    There's no need to use JS for highlighting a selected option. You can use radio buttons instead of divs with the same name so only one can be selected at a time.

    Then with CSS you hide them and style the labels, so whenever you select a label it selects the button and if you pick a different one, it deselects it.

    JavaScript is only needed to switch between Select and Thanks screens (eventListener) and to show the selected value (for loop).

    You can look at my solution for a better idea on what I mean.

    Hope this helps!

    Marked as helpful
    1
  • Saulius K.•560
    @TH3RIVPosted over 2 years ago

    Hi, @doyin156!

    Your project is not far from being finished, here's a few suggestions to make it even better:

    • Your page should contain landmarks, so it makes your code easier to read on what content is what and also helps screen-readers to figure out what is what. In this case your card should be wrapped withing main tags and your attribution should be wrapped within footer tags.
    • You are writing your CSS within the HTML, I would suggest making a separate file for CSS. Imagine this was a big website with a lot of styling and markup.
    • Your body is the wrong colour. You are using the text colour. Background should be Light gray: hsl(212, 45%, 89%).
    • You are using a h3 without a h1. Headings are supposed to follow the hierarchy (h1 > h2 > h3 > etc...). In this case it should be a h1 element. You can adjust the font size with CSS.
    • Try to avoid using margin to center your objects. For centering your card I would suggest looking up "Flexbox".
    • Try to avoid using px values as much as possible and use rem instead.

    Hope this helps!

    Marked as helpful
    0
  • Luba•20
    @Lubka024Submitted over 2 years ago

    CSS

    1
    Saulius K.•560
    @TH3RIVPosted over 2 years ago

    Hi, @Lubka024!

    Your card looks great!

    There's no need or is almost impossible to get it 100% accurate without the actual design files that give you all the margins and paddings etc, so don't worry about it as long as it looks close. Now onto a few suggestions for future improvement:

    • Your card should be wrapped within main tags, so it is easier to read the code and screen readers know what is the main content. Landmarks are important.
    • Try to avoid using px values and use rem instead.
    • Try to start using custom CSS properties. While it is not essential here, it is a good habit to develop.

    Hope this helps!

    Marked as helpful
    1
  • Saulius K.•560
    @TH3RIVPosted over 2 years ago

    Hi, @CarolinaVrl!

    Your card looks great! There's only a few suggestions I can make for future improvement:

    • Your attribution should be either wrapped within main tags or footer tags. That way screen readers will have easier time figuring out what it is.
    • Try to avoid using px values and use rem instead.
    • There's no need to declare a fixed with or height for the containers, max-width for the card is enough. The rest can be adjusted with margins or padding on the parent.

    Keep up the good work! Hope this helps!

    Marked as helpful
    1
  • Saulius K.•560
    @TH3RIVPosted over 2 years ago

    Hi, @iulias17!

    I believe it has something to do with position property on your <div class="avatar">. Why not keep in the middle of the card and then center the image within the div. Can't think of any other reason for that behaviour.

    Hope it helps!

    0
  • Saulius K.•560
    @TH3RIVPosted over 2 years ago

    Hi, @Nunamniroi!

    Your card is nearly done, it could use a few adjustments:

    • You should be using landmarks. This case your card is your main content, so it should be wrapped between main tags. Makes it easier to understand the code and helps screen-readers to figure out your main content.
    • There is no need to use header element as it is part of your card. Just wrap your whole card in a container element:
    <div class="card">
    <div class="first-section">
         Section code...
    </div>
   <div class="second-section">
         Section code...
   </div>
   <div class="third-section">
         Section code...
   </div>
</div>
    • Your button does not need a <p> element, just use text of whatever you want it to say.
    • You should not limit the width of your body, you can always limit the size of your elements within the body.
    • You can limit the width of your card using max-width property and you can center your card within the page using flexbox on your body element.
    • To round the card corners you use border-radius property. And if it's not showing, you use overflow: hidden.
    • Try to avoid using px values and use rem values instead.
    • Try using custom CSS properties for your projects, it is great habit to develop.

    Hope this helps! You got this!

    Marked as helpful
    1
View more comments