Skip to content
OverviewSolutions
32
Comments
692
👾 Ekaterine Mitagvaria 👾
@catherineisonline

All comments

  • 👾 Ekaterine Mitagvaria 👾•7,600
    @catherineisonlinePosted almost 2 years ago

    HI! Your solution looks nice though there are a couple of things you can improve which I hope will be helpful! 😊

    I would also add some validation for Lastname, Firstname, and Password length because accepting just 1 letter I guess isn't the best choice.

    Also, it gives me an error right away as soon as I start typing and never did form submit. I would change that logic probably.

    Besides, when I submit the form it shows me an error only for the first input and nothing for the rest. There should be errors for all the empty inputs after submission.

    I also see you don’t have README. README is a very important aspect of making projects, especially if you want other people to see it. As the name says, it’s the first thing people read when interacting with the project, it is kind of a manual. You can include many things there like the languages you used, which dependencies you installed, what was the process like, and what did you achieve or learn. Frontend Mentor also has a pretty nice README template which you can use to tailor the one depending on your preferences.

    3
  • 👾 Ekaterine Mitagvaria 👾•7,600
    @catherineisonlinePosted almost 2 years ago

    HI! Your solution looks nice though there are a couple of things you can improve which I hope will be helpful! 😊

    I would add some transitions for active states (when colors change on hover). It creates more interactivity and makes the project look cooler. Active states can be done on buttons, links, titles which act like links, or anything else, you choose.

    You can read more about it here, in case you haven’t done much of it:
https://www.w3schools.com/css/css3_transitions.asp

    The image with the alt "personal-finances", "banking-coverage", "consumer-payments" should be used more as a decorative image, rather than being attached to any content.

    Alt attribute for the image is important in order to specify alternative text for the image in case it will not be displayed. Using alt attribute is good for not only accessibility but also SEO and for situations when the image is loading too slowly. 
If the image is just for decoration you can still write an alt attribute but leave it empty, such images don’t need any alt tag but you will need to also add aria-hidden=“true”. What aria-hidden does is that it removes the entire element from the accessibility tree.

    If otherwise, you need to use an alt tag to describe the image. To write an alt tag you need to describe the content and purpose of the image and try not to use words like “picture of” or “image of”.

    Marked as helpful
    1
  • 👾 Ekaterine Mitagvaria 👾•7,600
    @catherineisonlinePosted almost 2 years ago

    HI! Your solution looks nice though there are a couple of things you can improve which I hope will be helpful! 😊

    I would add some transitions for active states (when colors change on hover). It creates more interactivity and makes the project look cooler. Active states can be done on buttons, links, titles which act like links, or anything else, you choose.

    You can read more about it here, in case you haven’t done much of it:
https://www.w3schools.com/css/css3_transitions.asp

    The image with the alt "Cart icon" should be used more as a decorative image, rather than being attached to any content.

    Alt attribute for the image is important in order to specify alternative text for the image in case it will not be displayed. Using alt attribute is good for not only accessibility but also SEO and for situations when the image is loading too slowly. 
If the image is just for decoration you can still write an alt attribute but leave it empty, such images don’t need any alt tag but you will need to also add aria-hidden=“true”. What aria-hidden does is that it removes the entire element from the accessibility tree.

    If otherwise, you need to use an alt tag to describe the image. To write an alt tag you need to describe the content and purpose of the image and try not to use words like “picture of” or “image of”.

    To make your container responsive you should change the media query for flex-direction because you set it to (min-width: 376px) but it doesn't fit the screen until 660px. So I would change flex-direction to row after 660px

    Marked as helpful
    2
  • YouLackHope•70
    @YouLackHopeSubmitted almost 2 years ago

    Hey there, rate it :). Anywhere i can improve? Let me know!

    Orde Summary!

    1
    👾 Ekaterine Mitagvaria 👾•7,600
    @catherineisonlinePosted almost 2 years ago

    Hello 🙌🏻 Your solution looks great however here are a couple of things you can improve which I hope will be helpful! 😎

    The image for the music icon should be used more as a decorative image, rather than being attached to any content.

    Alt attribute for the image is important in order to specify alternative text for the image in case it will not be displayed. Using alt attribute is good for not only accessibility but also SEO and for situations when the image is loading too slowly. 
If the image is just for decoration you can still write an alt attribute but leave it empty, such images don’t need any alt tag but you will need to also add aria-hidden=“true”. What aria-hidden does is that it removes the entire element from the accessibility tree.

    If otherwise, you need to use an alt tag to describe the image. To write an alt tag you need to describe the content and purpose of the image and try not to use words like “picture of” or “image of”.

    The a tag you use for "Change" should be a button.

    Buttons are useful when you want to create some interactivity on the same page, for example, a pop-up modal or form submission. Definitely, this is not a big project and this button has no purpose right now however and it could potentially be a tag as well but less likely, it’s good to have that habit of differentiating buttons and <a> tags😊

    Also, make sure to wrap the entire code in the main tag instead of using the div, the one where the class is “card”. It will help to remove report issues and improve accessibility as well. If you are using, for example, header or footer tags, you can place them outside the main tag.

    Marked as helpful
    1
  • 👾 Ekaterine Mitagvaria 👾•7,600
    @catherineisonlinePosted almost 2 years ago

    HI! Your solution looks nice though there are a couple of things you can improve which I hope will be helpful! 😊

    Make sure to wrap the entire code in the main tag instead of section. It will help to remove report issues and improve accessibility as well. If you are using, for example, header or footer tags, you can place them outside the main tag.

    I believe the text's Reliable, efficient delivery & Powered by Technology should be the same text and wrapped in h1.

    The images on the cards should be used more as a decorative images, rather than being attached to any content.

    Alt attribute for the image is important in order to specify alternative text for the image in case it will not be displayed. Using alt attribute is good for not only accessibility but also SEO and for situations when the image is loading too slowly. 
If the image is just for decoration you can still write an alt attribute but leave it empty, such images don’t need any alt tag but you will need to also add aria-hidden=“true”. What aria-hidden does is that it removes the entire element from the accessibility tree.

    If otherwise, you need to use an alt tag to describe the image. To write an alt tag you need to describe the content and purpose of the image and try not to use words like “picture of” or “image of”.

    0
  • 👾 Ekaterine Mitagvaria 👾•7,600
    @catherineisonlinePosted almost 2 years ago

    HI! Your solution looks nice though there are a couple of things you can improve which I hope will be helpful! 😊

    For containers with classes "container-two", "container-three" and "container-four" you don't need the border. Instead, you can remove the border and set a box shadow:

    box-shadow: -1px 0px 5px -2px hsl(208, 11%, 55%);

    Also, make sure to wrap the entire code in the main tag. It will help to remove report issues and improve accessibility as well. If you are using, for example, header or footer tags, you can place them outside the main tag.

    Make sure to use rem (relative length value) or em units instead of pixels for the purpose of “respecting the user preferences”, to say so. It makes the “sizes” of the website fluid according to the zoom/sizes set by the user. I see you do use rem sometimes but sometimes you use px for font-size, so make sure to stick with one thing.

    You can read more about it here: https://www.freecodecamp.org/news/what-is-rem-in-css/

    Marked as helpful
    0
  • 👾 Ekaterine Mitagvaria 👾•7,600
    @catherineisonlinePosted almost 2 years ago

    HI! Your solution looks nice though there are a couple of things you can improve which I hope will be helpful! 😊

    I would also add some transitions for active states (when colors change on hover). It creates more interactivity and makes the project look cooler. Active states can be done on buttons, links, titles which act like links, or anything else, you choose.

    You can read more about it here, in case you haven’t done much of it:
https://www.w3schools.com/css/css3_transitions.asp

    The image with the alt "car-icon" should be used more as a decorative image, rather than being attached to any content. Also, you wrote alt only on two images and the third one is empty.

    Alt attribute for the image is important in order to specify alternative text for the image in case it will not be displayed. Using alt attribute is good for not only accessibility but also SEO and for situations when the image is loading too slowly. 
If the image is just for decoration you can still write an alt attribute but leave it empty, such images don’t need any alt tag but you will need to also add aria-hidden=“true”. What aria-hidden does is that it removes the entire element from the accessibility tree.

    If otherwise, you need to use an alt tag to describe the image. To write an alt tag you need to describe the content and purpose of the image and try not to use words like “picture of” or “image of”.

    Finally, for the images to match width you can also do:

    
.icon {
  width: 30%;
  height: 80%;
}

    But also better to set some specific width and height instead of percentage.

    0
  • 👾 Ekaterine Mitagvaria 👾•7,600
    @catherineisonlinePosted almost 2 years ago

    HI! Your solution looks nice though there are a couple of things you can improve which I hope will be helpful! 😊

    First fo all you need to have a heading, h1 in this case. You should not use only p tags everywhere.

    <h1>Improve your front-end skills by building projects.</h1>

    To separate them there are different ways but you can just use <br> (break) for example between the text. I usually just add padding and the text moves itself.

    Also, make sure to wrap the entire code in the main tag, so outside div where the class is “div” add <main> tag. It will help to remove report issues and improve accessibility as well. If you are using, for example, header or footer tags, you can place them outside the main tag.

    And also to center your div with the class "div" add and remove this margin-left.

    margin: 0 auto;
``
    0
  • 👾 Ekaterine Mitagvaria 👾•7,600
    @catherineisonlinePosted almost 2 years ago

    Hello 🙌🏻 Your solution looks great however here are a couple of things you can improve which I hope will be helpful! 😎

    The way you are changing images with id desktop and mobile looks interesting but there is a more simple way to do that. You can either simply set it in CSS using media query. So let's say you want to use a mobile image when the screen width is less than 700px you do

    
@media screen and (max-width: 700px) {
image {
content: url("/assets/images/image-web-3-mobile.jpg")
}

}

    Or there is another way without media queries and also you can set several images for different screens. I recommend reading it here and experimenting with this, I have started using it recently, and sounds like a great solution as well:

    https://developer.mozilla.org/en-US/docs/Learn/HTML/Multimedia_and_embedding/Responsive_images

    Make sure to use rem (relative length value) or em units instead of pixels for the purpose of “respecting the user preferences”, to say so. It makes the “sizes” of the website fluid according to the zoom/sizes set by the user. I see you do use it but you use px, em, and rem at the same time. Was it intended?

    You can read more about it here: https://www.freecodecamp.org/news/what-is-rem-in-css/

    Marked as helpful
    0
  • vabel17•120
    @vabel17Submitted almost 2 years ago

    Other than the messy code, there is one thing I'd like to change but don't know how. On smaller viewports my bottom margin is not visible and my elements seem to overflow their main container, why is that? Thanks!

    Testimonials grid section | css grid

    1
    👾 Ekaterine Mitagvaria 👾•7,600
    @catherineisonlinePosted almost 2 years ago

    HI! Your solution looks nice though there are a couple of things you can improve which I hope will be helpful! 😊

    First of all, what do you mean by "my elements seem to overflow their main container, why is that?". What are YOUR ELEMENTS?

    To fix the height issue instead of writing height: 100% for the body change it to min-height: 100vh.

    html {
    height: 100%;
}
body {
    min-height: 100%;
}

    This allows the HTML element to reference the parent viewport and have a height value equal to 100% of the viewport value. With the HTML element receiving a height value, the min-height value assigned to the body element gives it an initial height that matches the HTML element.

    Try to use sections instead of just divs, div has no semantic meaning and semantics is actually a very useful thing for accessibility.

    I also see you don’t have README. README is a very important aspect of making projects, especially if you want other people to see it. As the name says, it’s the first thing people read when interacting with the project, it is kind of a manual. You can include many things there like the languages you used, which dependencies you installed, what was the process like, and what did you achieve or learn. Frontend Mentor also has a pretty nice README template which you can use to tailor the one depending on your preferences.

    Marked as helpful
    0
  • 👾 Ekaterine Mitagvaria 👾•7,600
    @catherineisonlinePosted almost 2 years ago

    HI! Your solution looks nice though there are a couple of things you can improve which I hope will be helpful! 😊

    Heading levels should only increase by one. This means that when you have several headings they should downgrade by one. So, you start with h1, then h2, and then h3 and not h5 right away, for example. If you are repeating the same headings, it's okay. That doesn't mean every next heading needs to be different, beside the h1 (it can only be used once).

    Also, make sure to wrap the entire code in the main tag instead of using the div, the one where the class is “main”. It will help to remove report issues and improve accessibility as well. If you are using, for example, header or footer tags, you can place them outside the main tag.

    Also, I recommend setting the max-width for the container so it doesn't;t stretch too much on bigger screens. When you set for example max-width: 30rem; the container will never become larger than 30rem. If you set only % it will keep stretching as the width grows. The button looks not so pleasing after 1440px

    Marked as helpful
    0
  • 👾 Ekaterine Mitagvaria 👾•7,600
    @catherineisonlinePosted almost 2 years ago

    Congrats on the solution☀️ I have also done this one and enjoyed it a lot. Here are a couple of things you can improve which I hope will be helpful! 👍

    First of all, don't use a tag for "Proceed to Payment" or "Cancel Order". They are not links, it's buttons. Buttons are useful when you want to create some interactivity on the same page, for example, a pop-up modal or form submission. Definitely, this is not a big project and this button has no purpose right now however and it could potentially be a button as well but less likely, it’s good to have that habit of differentiating buttons and <a> tags 😊 As you see yourself, you left href attribute empty because you didn't need it right? You are not redirecting anywhere.

    Also, make sure to wrap the entire code in the main tag instead of using the div, the one where the class is “form”. It will help to remove report issues and improve accessibility as well. If you are using, for example, header or footer tags, you can place them outside the main tag.

    The image with the alt "icon-music" should be used more as a decorative image, rather than being attached to any content.

    Alt attribute for the image is important in order to specify alternative text for the image in case it will not be displayed. Using alt attribute is good for not only accessibility but also SEO and for situations when the image is loading too slowly. 
If the image is just for decoration you can still write an alt attribute but leave it empty, such images don’t need any alt tag but you will need to also add aria-hidden=“true”. What aria-hidden does is that it removes the entire element from the accessibility tree.

    If otherwise, you need to use an alt tag to describe the image. To write an alt tag you need to describe the content and purpose of the image and try not to use words like “picture of” or “image of”.

    0