Skip to content
OverviewSolutions
12
Comments
152
Dharmik_487
@Dharmik48

All comments

  • Dharmik_487•1,740
    @Dharmik48Posted about 3 years ago

    Hey👋,

    Good job with the solution! Looks nice, I just have a couple of things I think you can improve:

    • At around 930px screen width, the form looks very thin, so I think you should increase the width a bit.
    • Also it would be good if you add transition on hover state.

    Keep Developing👍

    0
  • Dharmik_487•1,740
    @Dharmik48Posted about 3 years ago

    Hey👋,

    Your solution looks pretty good! I just have a couple of suggestions:

    • Firstly, there is a vertical scroll bar in your solution and I don't think it should be there.
    • And I suggest you to remove the default outline on focus state from buttons and links.

    Apart from these you solution looks nice, keep it up👍

    Marked as helpful
    1
  • CoderPr0•645
    @CoderPr0Submitted about 3 years ago

    This is my next work. I struggled a bit with the Javascript bit, but I think I did a decent job. As always any tips or feedback would be appreciated.

    Time-tracking-dashboard

    1
    Dharmik_487•1,740
    @Dharmik48Posted about 3 years ago

    Hey👋,

    Good job on completing the challenge! I just have one suggestion for you:

    • Try to use relative units like em and rem more instead of px as it is a good practice and helps in responsive design as well!

    Keep Developing👍

    Marked as helpful
    1
  • Dharmik_487•1,740
    @Dharmik48Posted about 3 years ago

    Hey👋,

    Great job with the solution! Looks great, but.. I just have one suggestion for you:

    • Try to use relative units like em and rem more instead of px.

    Keep Developing👍

    Marked as helpful
    0
  • Dharmik_487•1,740
    @Dharmik48Posted about 3 years ago

    Hey👋,

    Your solution is really good! But.. I found a few suggestions:

    • Try to use semantic html tags like main, section, header, etc. more as it is a good practice and good for SEO.
    • Also, I think you should remove the margin-top: 10rem; from 886px media-query as because of it there is a vertical scrollbar.

    Keep it Up👍

    Marked as helpful
    0
  • Jonathan•295
    @Nathan-FrontSubmitted about 3 years ago

    Updated version: Any suggestions on the javascript of the feature area of this challenge? Not really sure if my javascript on this is good or not?

    Book mark landing page

    1
    Dharmik_487•1,740
    @Dharmik48Posted about 3 years ago

    Hey👋,

    Your solution looks good, but.. I have a few suggestions:

    • In the features section, when I try to click on the other feature, the details below won't change. So try to add it.
    • Also it would be great if you add some transition to hover state.
    Marked as helpful
    0
  • Dharmik_487•1,740
    @Dharmik48Posted about 3 years ago

    Hey👋,

    Your solution looks good, but.. I have found a couple of issues:

    • You have not add any form validations, so add it.
    • And also add some hover effect with transition to the buttons and links

    Keep it up👍

    Marked as helpful
    1
  • Shanib Ibrahim•160
    @shanib-ibrahimSubmitted about 3 years ago

    I have completed another challenge using js for the first time, I appreciate your feedback to continue improving my code to keep growing

    HTML,SASS AND JS

    1
    Dharmik_487•1,740
    @Dharmik48Posted about 3 years ago

    Hey👋,

    Your solution looks great! Maybe just add some transition to hover effect.

    Keep it up👍

    Marked as helpful
    0
  • MHIEN•230
    @MinHien-gitSubmitted about 3 years ago

    i want to center the card without using position absolute and translate(-50%,-50%)

    css and HTML

    1
    Dharmik_487•1,740
    @Dharmik48Posted about 3 years ago

    Hey👋,

    • To center the card you can use display: flex; and place-items: center; to the parent.

    Hope it helps🤗

    0
  • Dharmik_487•1,740
    @Dharmik48Posted about 3 years ago

    Hey👋,

    Good job on completing the challenge! But.. I have a few suggestions:

    • On the buttons, at normal state, there is no border, but when hovered there is a border. So, when I hover it the content just pops up because of the border. To fix that maybe add a transparent border at normal state.
    • Also it would be good if you add some transition on hover state.
    0
  • Dharmik_487•1,740
    @Dharmik48Posted about 3 years ago

    Hey👋,

    Your solution is really Good! I just have one suggestion for you:

    • Add some transition to hover effects, as it will increase the experience.

    Keep it Up👍

    0
  • Dharmik_487•1,740
    @Dharmik48Posted about 3 years ago

    Hey👋,

    Your solution is really good! Looks great, but.. I just found one issue:

    • Below screen width of 375px the cards all become aligned like on bigger screen instead of like it should be on small screens.

    Apart from these, it looks really good, Keep it Up👍

    Marked as helpful
    0