Skip to content
OverviewSolutions
53
Comments
30
P
Krishna Vishwakarma
@KrishnaVishwakarma1595

All comments

  • Krishna Vishwakarma•1,390
    @KrishnaVishwakarma1595Posted 5 months ago

    Hi Sva

    I have a few points to mention -

    • As I saw on the code, you've given a common id="socials" to all the a links. This should not be a common ID of an element that can be unique. Instead, you should use class="socials"
    • Also, I don't know why clicking on the buttons doesn't redirect me to the given link. It doesn't act at all. It is just changing the class active to clicked button.
    0
  • Alessandro-jpg•70
    @Alessandro-jpgSubmitted about 1 year ago

    This is my third project any feedback is welcome i have a doubt i need to know how i can make to have that color when i open the nav_menu that black color and any advice about the responsive design is welcome im still learning about it

    News-Homepages

    1
    Krishna Vishwakarma•1,390
    @KrishnaVishwakarma1595Posted about 1 year ago

    Hi, @Alessandro-jpg

    I've some points to mention that can help you to improve your solutions.

    • For the main logo image <img src="assets/images/logo.svg"> you should provide alt="Logo". Keep alt="" empty for images for icons.

    • You missed the hover state for the top navbar menu, the side container beside the main content where other news blogs are listed and the bottom three articles.

    • You provided overflow: hidden in the body tag, which prevents the user to scroll vertically. Either remove it or provide overflow-y: auto.

    • You need to work on spacing between content.

    • Use Modern CSS Reset

    • For the mobile menu background black overlay navigation you checkout this Responsive full screen menu or checkout my solution.

    I hope these points will help you to improve your solutions.

    Happy Coding

    0
  • Krishna Vishwakarma•1,390
    @KrishnaVishwakarma1595Posted about 1 year ago

    Hi, @Afrid-Ahamed

    I've got a few recommendations for you to improve -

    • Start using Modern CSS Reset rules so you don't need manual reset. Just take the code from there.

    • Start using HTML5 Semantic Elements like <header>, <main>, <section>, etc. with HTML Accessibility rules.

    • For the pattern divider image you can provide alt="" empty. alt="pattern-divider-desktop" doesn't imply anything to screen readers. Provide alt="" with some values for main content-related images. For icons, we can keep it empty.

    However, the solution looks great. Good job! Hope this help you to improve.

    Happy Coding

    0
  • Krishna Vishwakarma•1,390
    @KrishnaVishwakarma1595Posted about 1 year ago

    Hey, @RedMwp

    I have got a few recommendations to improve -

    • You should use <form> tag instead of <div>.

    • When we work with forms it should be mandatory for us to implement the error handling too. Like right now, it doesn't show any error message or something when we either submit and empty form or form with any wrong values for email field. I think you should work on error handling. Show the appropriate message for the empty field or field with wrong value. That's a good practice when working with the forms.

    • When focusing on the input, you can keep the outline border transparent or none. outline: none

    • Instead of giving width: 90% to the button you can keep it 100% & provide the padding like this padding: 1em 2rem for your .form class. So it will look nice.

    Hope this will help you to improve. Keep Mentoring!

    Happy Coding

    Marked as helpful
    2
  • Krishna Vishwakarma•1,390
    @KrishnaVishwakarma1595Posted about 1 year ago

    Hi, @Gilocah28

    Nice solution. Good job. However, I have got a few recommendations that can help you to improve.

    • We can enter the text also. However, you have an error message for this. Instead of input type"text" you should use input type="number". So, your extra step to checking whether the input is text or number will reduce. You can keep it like this -

    <input type="number" id="day" min="1" max="31" /> Min and max attribute prevent user to enter a negative value using keyboard's arrow down/up key. And it keeps the input value range between 1-31 for the day field.

    • I noticed when we fill in the input with any value and we focus the input again, it automatically clears the input.

    Happy Mentoring

    Marked as helpful
    1
  • Krishna Vishwakarma•1,390
    @KrishnaVishwakarma1595Posted about 1 year ago

    Hi, @Demils13

    Your QR code image is not loading. For that, you can provide the src like this <img src="./images/image-qr-code.png" alt="QR Scan Code" />.

    ./ locates the current directory you are working on. Like @DundeeA said in his comment he misses the .. If you provide like that <img src='/images/image-qr-code.png' /> then also it won't load the image, cause it doesn't understands the path of file to locate.

    • We must always provide the alt="" attribute to the <img> tag. If it used for icons or non-describing images the we can keep the alt="" tag empty.

    • You should start using HTML Semantic Elements like <header>, <main>, <section>,<footer> etc. With these you'll understand the HTML Accessibility rules to keep.

    I hope this will help you. Nice solutions. Good job. Congratulations on completing this challenge.

    Happy Mentoring

    Marked as helpful
    2
  • Krishna Vishwakarma•1,390
    @KrishnaVishwakarma1595Posted about 1 year ago

    Hi, @defydef

    Nice solution. Good job. Although, I've some recommendations that can improve your solutions more -

    • The user can fill the negative values to the inputs using keyboard arrow down/up keys. To prevent this you can provide min="1" attribute to inputs so user can enter negative values.

    • We can't able to focus on the input by clicking on the label. However, labels have for="" attribute with the values of the input's name attribute value. Instead, you should give id="day" also to make this possible.

    • After the submission, you can reset the input fields so that user don't need to clear them manually again.

    Hope these points will help you. The solution looks really good.

    Happy Coding

    0
  • Krishna Vishwakarma•1,390
    @KrishnaVishwakarma1595Posted about 1 year ago

    Hi, @newNASA

    Nice solution. I've few recommendations for you -

    • You should add form validation on first step. As, we can enter wrong email format and in phone number we can enter texts also. For email validation, you can use any email validation regex and for phone number field you can use input type="number" with minlength=12. So, it will allow user to enter numbers only.

    • Or for the phone number you can keep the input type="text", and on the keypress event listener you can check to allow only numbers and the plus icon. As sometimes, we can enter phone numbers with country codes like this +041, +91.

    • You can do the calculation part too. As I can see in step 4 it has just a static value. So, keep a calculation on previous steps of what plan and add-ons the user selects and on the finishing step show the correct calculation.

    • On the confirmation button, you should also show the Thank you message. I think you've skipped that part.

    I hope these points will help you to improve your solution.

    Happy Coding

    Marked as helpful
    1
  • Zdravko•420
    @Zdravko93Submitted about 1 year ago

    I liked this challenge, and had lots of fun buliding it. The biggest challenge for me was code refactoring, making it look more readable and splitting code into modular pieces, while trying not to repeat myself.

    Age Calculator App / CSS and Vanilla JavaScript

    1
    Krishna Vishwakarma•1,390
    @KrishnaVishwakarma1595Posted about 1 year ago

    Hello, @Zdravko93

    I've some points for you to improve the solution -

    • Your body has overflow: hidden and because of that I'm not able to scroll because some of the part of the container is hidden. Instead, provide overflow-y: auto to the body.

    • That's nice that you provided the modal for the error message. But, If we try to submit an empty form it shows the modal, as well as the error message for the day field. I think if you want to show the error message too, then show it for all fields otherwise don't show them just mark the field border red.

    • If I fill the day field with a valid value for example: 15 and hit submit button. It shows the error modal and the error message as well for the day field like "Must be a valid date", instead it should show the message for other fields, cause the day field has the valid value. I think you must check the error handling once.

    • You can provide a min="1" attribute to the input fields so the user won't be able to decrease the values to negative values using the keyboard arrow down/up keys.

    I hope these points will help you to make your solution better. Keep mentoring!

    Happy Coding

    Marked as helpful
    0
  • Krishna Vishwakarma•1,390
    @KrishnaVishwakarma1595Posted about 1 year ago

    Hey, @romannery

    Congrats on completing the challenge. Your solution looks great! I've some points for you

    • When we submit an empty form, the email field gets the input filled automatically with this value email@example/com. I think that should be empty too like other fields.

    • If there are no labels for the input then we should provide the label by the aria-label attribute for the inputs.

    • Although the .error-msg is hidden using display: none, but it could be read by screen readers, you must provide an aria-hidden="true" to all span for error messages.

    • When we fill in all the inputs and hit the submit button, you should reset the form and show the successful alert message using javascript alert() or you can create your custom message and show it.

    I hope these points will help you to improve. Nice solution. Keep mentoring!

    Happy Coding

    Marked as helpful
    1
  • Krishna Vishwakarma•1,390
    @KrishnaVishwakarma1595Posted about 1 year ago

    Hey, @Smallz97

    I have some points for you to improve the solution -

    • Instead of providing the background to your #hero section, you can provide the background to the body like below, so it won't look stretched.
    body{
    background: #121725 url(assets/desktop/image-host.jpg);
    background-position: center right;
    background-repeat: no-repeat;
    min-height: 100vh;
}

    • Also, the logo looks stretched so don't provide its width & height to the class .logobox, even in the media queries.

    • For the dotted pattern image you can provide CSS like below, so that also not get streched

    .dotted-pattern-box{
    position: absolute;
    width: 232px;
    height: 104px;
    right: 0;
    bottom: 70px;
}
.dotted-pattern-box img{
width: 100%;
    height: 100%;
    object-fit: cover;
}

    • When we focus into the input .email-input there is an outline, you can hide hit by giving outline:none

    • Even If I input the email like this krishna@gmail, it takes and didn't provide the error message that it is invalid.

    • Instead of the browser's default required message you can provide your custom one.

    • Don't provide width & height to these classes .supporting-brands, .brand & remove position: absolute from .brand img. It makes logos stretched.

    Hope these point will help you to improve the solution.

    Happy Coding

    1
  • Krishna Vishwakarma•1,390
    @KrishnaVishwakarma1595Posted about 1 year ago

    Hello, @mreed4

    Nice solution and congratulations on completing the challenge. I've some points for you -

    • You don't need to provide margin: auto to your .container class. Instead, you can provide this below CSS to your body so the container will be centred vertically & horizontally.
    justify-content: center;
align-items: center;

    @media screen and (max-width: 375px){
.pricing-and-why-us {
    flex-direction: column;
}
}
    • Instead of max-width: 375px, you should give max-width: 700px, cause between 375 to 700px the design is not responsive and getting overflowed from the screen. Also, you can make the .container class width:100%.

    I hope these points will help you.

    Happy Coding

    Marked as helpful
    1