Skip to content
OverviewSolutions
31
Comments
21
Alucard
@Alucard2169

All comments

  • Alucard•870
    @Alucard2169

    hi, you did a great job in this challenge, there are some issues, one of them is the share button (when the user clicks the share button , it opens up the share menu, but if the user decides to not use it, they would have to reload the browser to close the share menu)

    if added a state functionality, it will keep the current state of the share menu ( if it's open or not), i have explained it in the pull request on github.

    hope it help, if you have any other issues, just ask

    keep it up

    Marked as helpful
  • Alucard•870
    @Alucard2169

    hi, i fixed some of the issues, i have explained them in the pull request, hope it helps

    avoid using inline css

    overall you did a very good job

    keep it up

  • Alucard•870
    @Alucard2169

    Hi, Great job on the design,

    next you should try to make it interactive and functional, as it will give you tons to learn on the way.

    Here is how you can fix your HTML and Accessibility errors:

    • you should always change heading size by one, for example ( a page must have a H1 heading (very important), after that you should decrease the size by one like h2 > h3 > h4 ...., don't just jump to h3 or h4 );

    • Input element must have a label all times, here is how you can do that

     <label for="{your input's ID}">   *some_text*    </label>
 <input id="{your input's ID}" >

    hope that helps

    Marked as helpful
  • Alucard•870
    @Alucard2169

    hey @yurideoliveira,

    Great job you design looks great,

    to solve the accessibility issue, you should use at least one <h1> element in you page.

    Happy coding!

    Marked as helpful
  • Alucard•870
    @Alucard2169

    hi @luisoliverosrdn,

    You did a awesome job.

    • here are some points to help you:-

    -- use display: flex and a flex-direction of column with height 100vh on body, to center the card perfectly.

    -- you don't have to use alt attribute on anchor tag.

    happy coding!

    Marked as helpful
  • Alucard•870
    @Alucard2169

    Hi,

    • Design look good, but a bit too small, so you should work on the dimensions to make it more visible and stand out on the screen.

    • The formula you used to calculate the tip and total amount aren't working properly, you can look up online to see the correct formula for tip-calculator.

    • The reset button is suppose to clear every input and amount display, and bring everything back to square one. You should work on that.

    • People input isn't suppose to take negative or fraction values.

    • Bill input and custom tip input isn't suppose to take negative values.

    • Using attributes like hiddenheading and zero-people on html element is not valid, if you want to set something to hidden you can use CSS or JS for it.

    Best of Luck

  • Alucard•870
    @Alucard2169

    hi @Nazemrap great work.

    For centering the card property with the body

    you can put your card in a container and then use display: flex on the body, remember to set a height on your body of 100vh and then use justify-content and align-items property.

    Flexbox and grid are very important to make responsive designs. and they also make the work a lot easier.

    here this video gives a great understanding of flexbox

    Marked as helpful
  • Alucard•870
    @Alucard2169

    Hi @ojitxslml nice work, one thing you can add is a error when you type. Fraction in people count input cause people can't be in fraction as you know, all in all its a nice project.

    Also you can work on the sizing cause if the user types a big enough number the while calculator exeeds it's size limit in mobile version.

    Keep up the good work

  • Alucard•870
    @Alucard2169

    Hi there @FelipeFama Great job on this project 🎉🎉🎉

    Here are a few things you can improve:-

    • Your card is a bit too big in height, you can work on that.

    • That will solve the scrolling issue, if it doesn't just use overflow:hidden on the body, but make sure that your card is fully visible.

    • To center your card you can use display:flexon body

    • Also keep in mind that you can't actually write your content directly inside the <div> tag, div tag is use to divide our content into parts, you have to write your content inside tags like <p>,<h1>,<article>,<aside> which are made to store content.

    Nice project, keep it up 😊😊😊

    Marked as helpful
  • Alucard•870
    @Alucard2169

    Hi @agausha great job finishing this challenge 🎉🎉🎉🎉

    There are a few things you can improve:-

    • when you open your site, the card ( the whole ) isn't centered completely. You can use flex on your body and then center your card using that.

    • To fix your accessibility issues just do <main></main> instead of <div class="main"> and remember to create a new report 😊😊.

    You did Great, Keep it up 😊😊😊

    Marked as helpful
  • Alucard•870
    @Alucard2169

    @azenetesc really awesome work 🥳🥳🥳

    You can make it better by adding a little shadow to your card

    Keep up the good work 😊😊😊

    Marked as helpful
  • Alucard•870
    @Alucard2169

    Hi there @Rashmi-Wijesekara Nice work on this project 🎉🎉🎉

    It looks good so far,

    • Next thing you can do is, add a hover effect on the image.

    • The mobile version is not completely centered (Vertically), so you can do something about that.

    • The Desktop version is too close to the top, you can either use flex on body or just add some margin-top to the card, but i would suggest using flex.

    • there are some issues with the accessibility and that's cause

    • in your HTML file, a image tap must have an alt attribute. It will describe the image to the client in case the image is not loading.

    I hope this helps you.

    Nice work, Keep It Up 🥳🥳🥳

    Marked as helpful
  • Alucard•870
    @Alucard2169

    Hi @hhhameem Great job finishing this challenge.🥳🥳

    Your design looks fantastic 😊. One thing you can do is to center your card vertically, it's kinda too close to the top area.

    Also you take look up and do something about the accessibility issues it will give you more information about accessibility in general and things you should look out for.

    Awesome Design, Keep it up

    Marked as helpful
  • Alucard•870
    @Alucard2169

    @a2uuz really nice project, keep up the good work 😊😊

  • Alucard•870
    @Alucard2169

    Hi there @ryuuwiz Great work on this project, Next thing you can do is to add hover effects that will look really nice i it will also help you understand important design concepts.

    Also, you can do something to solve your Accessibility issues.

    Really Nice work, Keep It up 😊😊

    Marked as helpful
  • Alucard•870
    @Alucard2169

    Hi there @szymKamil awesome work on this project

    Here are few things that can be improved

    • the Desktop design should have a height of you screen and should not be scroll able. to achieve this you can use display:flex on your body and put all of you content inside a container and align it in the center of the screen.

    • this video provides a solid grasp on flexbox.

    • To hide the overflow you can use overflow: hidden

    • while making a div you should write content inside it without any tag for example <div>text</div> as this is not best practice and also not valid.

    • I have done this challenge so if you want you can go look at it.

    Keep up the good work 😊

  • Alucard•870
    @Alucard2169

    for Accessibility errors :

    • consider using just <html lang="en"> the browser recognizes everything by default so you don't have to use dir or data-cmp, also you don't use class attribute on html tag cause you don't have to select it for anything and it is also a accessibility and syntax error.

    • Viewport tag: by setting the maximum-scale and minimum-scale to 1 you are basically disabling the zooming which is very bad for you site. <meta name = "viewport" content="width = device-width, initial-scale = 1"> this is how it should be.

    • your html document should at least contain one <h1> heading element.

    • if you are using class main on your div then you should avoid using div for this and just use main as your html tag. You can learn more about that on semantic html.

    Marked as helpful
  • Alucard•870
    @Alucard2169

    Hi, great work on this project.🥳🥳

    for changing the color of your button on hover you have to use pseudo class on your button like this:

    button:hover{
   background: your_background_color;
   color: your_text_color;
}

    not only that, you can change many things on hover or using pseudo class ( trust me pseudo class are very fun )

    This Video gives a good grasp on hover This lesson from W3schools gives a brief introduction to pseudo classes.

    Anyway Great work out there, Keep it up.😊