Skip to content
OverviewSolutions
36
Comments
25
Dinesh
@Dinesh1042

All comments

  • Dinesh•1,115
    @Dinesh1042Posted almost 3 years ago

    Hi NGUYEN!

    Great work on this solution. Here are my few suggestions.

    • I think you haven't removed margin form the body looks like a white border.
    • Rating number are not accessible by the keyboard.
    • Adding transition on submit button would be better.

    Awesome work Keep going and happy coding.

    1
  • Dinesh•1,115
    @Dinesh1042Posted about 3 years ago

    Yes, you can but avoid using pixels most often use rem or em on font size. You can take on this StackOverflow question Should I use px or rem value units in my CSS? .

    Fantastic Work Happy Coding!

    Marked as helpful
    2
  • Dinesh•1,115
    @Dinesh1042Posted about 3 years ago

    Hi, ccreusat! Something unique makes people outstand. Fantastic on this challenge.

    Happy Coding!

    Marked as helpful
    1
  • Dinesh•1,115
    @Dinesh1042Posted about 3 years ago

    Hi Thiago, Something went wrong with your deployment. It shows our friend 404 error. Try deploying again on the root folder of the project.

    0
  • Dinesh•1,115
    @Dinesh1042Posted about 3 years ago

    Hi Ganesh! Great work on this challenge. I think you haven't seen the style readme file on the downloaded folder where they provide font-family and color for the project.

    Great work Happy Coding.

    Marked as helpful
    0
  • Dinesh•1,115
    @Dinesh1042Posted over 3 years ago

    Hello Sakssingh, 👋 Amazing work on your first challenge. Here is my small suggestion

    • Just take a look at your report of your project for accessibility issues

    Great work Keep Coding, Happy Coding🤗

    Marked as helpful
    1
  • Dinesh•1,115
    @Dinesh1042Posted over 3 years ago

    Hi, Wack04 Fantastic work on this challenge

    Here are my few suggestions

    • Consider using only one h1 tag as the top-level-heading only. The h1 tag is the most important heading because it’s the highest level tag that shows what your specific page is about.
    • The presence of more than one H1 won’t necessarily confuse the search engine, but it could dilute the SEO power of a single H1.
    • responsiveness of the site is great
    • Add :focus pseudo class to interactive elements like anchors, buttons, etc. Use outline property to make your website more accessible to keyboard users. Focusable elements like anchor, buttons, or inputs they have applied default :focus pseudo class with outline property. These default styles are subtle and hardly visible tho. Furthermore, every browser has a slightly different default style for the outline, so you probably want to change the default style.

    Great work Happy Coding and keep coding

    0
  • Dinesh•1,115
    @Dinesh1042Posted over 3 years ago

    Hello Sarajlija👋

    Fantastic work on this challenge Here are my few suggestion for your solution

    • In mobile view your footer has been squashed.
    • try adding some more padding on the article card and align your text center.
    • Looks great at desktop view 😋.

    That's it Great stuff. Happy Coding and Keep Coding🤗

    0
  • Dinesh•1,115
    @Dinesh1042Posted over 3 years ago

    Hello, tenzind12 👋

    Great work on this challenge, Here are my few suggestions on your solutions.

    • Tip Calculator works only when the user clicks on the select tip. If the user changes the bill amount or number of people. It's not updating.
    • There is no indication which tip is currently selected use classes or radio inputs to manipulate dom
    • Your .p1 and .p2 font-size are too small and slim

    Well, that's it. Happy Coding!

    Marked as helpful
    1
  • Dinesh•1,115
    @Dinesh1042Posted over 3 years ago

    Hi ESMITH!

    Fantastic work on this challenge 🙌 . Your solution responds well. Here is are my minor suggestions for your solution

    • When I resized my screen to tablet size your .card fits the entire width of the screen you can use max-width for that issue.
    • Your .card__info__description font-size seems to bit smaller. It makes harder to read for poor vision peoples

    Well, that's it. Happy Coding!

    Marked as helpful
    1
  • Dinesh•1,115
    @Dinesh1042Posted over 3 years ago

    Hello, TrakaMeitene!

    Fantastic work on this challenge. Here are my few suggestions

    • You have misspelled <!DOCTYPE html5> you should remove the 5.
    • Your .bildes overflows out of the window.

    You nailed it! Happy Coding

    0
  • Dinesh•1,115
    @Dinesh1042Posted over 3 years ago

    Great work on this challenge. You have managed the responsiveness of the site very well that's cool. Keep up the great work!

    1
  • Dinesh•1,115
    @Dinesh1042Posted almost 4 years ago

    Hello, saurabh! 👋

    Fantastic work on this challenge, Here are my few suggestion

    • You can add border-radius on the main element and make it overflow:hidden;. By that you can have the rounded edges in the card.

    • You should also make it responsive for smaller screens 📱.

    • Add :focus pseudo class to interactive elements like anchors, buttons etc. Use outline property to make your website more accessible to keyboard users. Focusable elements like anchor, buttons or inputs they have applied default :focus pseudo class with outline

    Good luck with that, have fun coding! 💪

    1
  • Dinesh•1,115
    @Dinesh1042Posted almost 4 years ago

    Hello, Talha!👋 Fantastic work on your first challenge, Here are my few suggestions

    • When you are trying to apply transition property for display:none to display:flex, transition properties do not work. The reason is display:none property used to remove element from the dom. display:flex property is used to make flexible box layout it cannot be partly displayed. That is why the transition property does not work. You can use keyframe animation @keyframes navAnimation{0%{display:none; opacity:0; } 1%{display:flex; opacity:0;}100% {opacity:1; }}
    • another way is using transform property set transform:translateY(-100%) property in mainMenu this moves mainMenu out of the screen. When the nav is in the active stage change the transform property to transform:translateY(0).
    • Consider using the h1 element as a top-level heading only the reason for that is h1 elements are treated as top-level headings for many screen readers.
    • Your .links should be accessible for screen readers use <a href=""></a> tag for all your social link and Add some aria-label for your .links. This attribute is used to define a string that labels the current element. In your case these anchors have icons inside so screen readers users won't know what it is.

    Wish you Happy Coding!💪

    0
  • Dinesh•1,115
    @Dinesh1042Posted almost 4 years ago

    Great Work on this challenge the calculator looks awesome.😍 I really liked the way of OOP in your code and I have learned something new from that. and Awesome work custom color selection.

    Happy Coding!💪

    1
  • Dinesh•1,115
    @Dinesh1042Posted almost 4 years ago

    Hello, MALLORY!👋 Fantastic work on this challenge.

    Here are my few suggestions.

    • When I resized my screen to tablet view your .card seems to be too small. I think you have used mediaquery at 375px .You should increase your media query width to make it responsive
    • You can use max-width in the .card. So it won't grow when it hit the certain point.

    Wish you Happy Coding.💪

    1
  • Dinesh•1,115
    @Dinesh1042Posted almost 4 years ago

    Hi, KHANG DUONG! Fantastic work on your first challenge

    • You can have a overlay on the image element and use mix-blend-mode:multiply on the overlay change the opacity to opacity:0.75; to make it look like design.

    • I Suggest you to have a look on Mediaquery documentation. to make it responsive

    Great work!😍 Happy Coding💪

    0
  • Dinesh•1,115
    @Dinesh1042Posted almost 4 years ago

    Hello, Andrej!👋

    Fantastic work on this challenge.

    Here are my few suggestions for your solution

    • You should increase your font-size in .paragraph to allow users easier to read
    • You should change font-family in .paragraph according to their style guide
    • When resizing to mobile view there is more breathing space on the top of the .btn. Instead of specifying min-width in .cars__item you can make your .btn as display:inline-block and add margin-top to that.

    Wish you Happy Coding💪

    0