Skip to content
OverviewSolutions
26
Comments
52
Raj Prasad Sanjel
@RajSanjel

All comments

  • Raj Prasad Sanjel•980
    @RajSanjelPosted over 2 years ago

    By using line-height property of css

    0
  • Raj Prasad Sanjel•980
    @RajSanjelPosted over 2 years ago

    Congrats on finishing your 1st projects.

    If you need any help you can contact me.

    Marked as helpful
    2
  • George Farg•170
    @GeorgeFargSubmitted over 2 years ago

    any suggestion is appreciated again

    flex

    1
    Raj Prasad Sanjel•980
    @RajSanjelPosted over 2 years ago
    • You forgot to add hover effects. You can see those effect design in design/active-states.
    • You haven't used semantic html. You can learn about it here.

    You have made it responsive and it looks great! Hope this was helpful.

    Marked as helpful
    0
  • Raj Prasad Sanjel•980
    @RajSanjelPosted over 2 years ago

    There is a bug in your website. When user click on 1 and then submit it doesn't submit.

    0
  • Raj Prasad Sanjel•980
    @RajSanjelPosted over 2 years ago

    Update your breakpoint width to 1000px it looks ugly at current breakpoint.

    0
  • Pon Huang•210
    @ponhuangSubmitted over 2 years ago

    My FIRST JavaScript challenge, it took me 4 hours to finish this. 😅 In the end, it seems to work well, however, there are 2 things I don't know how to make it work yet.

    1. I use toggle to apply "active" class when user clicks the rating value, but it didn't apply the CSS property in the end. (It should show and keep the primary orange color when the button is clicked.)

    2. How can we just show mainly the latest button with active class, when the user click it. They could click many times with different values, but how can we just show the latest clicked?

    If you know how to solve the problems or any idea, welcome to give me any advice, thanks for helping. :)

    Interactive Rating Component - CSS Design and JavaScript

    1
    Raj Prasad Sanjel•980
    @RajSanjelPosted over 2 years ago
    • You should style active class in css.
    • You can add if else for removing the active class from previously clicked button.
    • When user doesn't click anything and submit the score it shows "You selected out of 5" you should set it 1 by default or add alert saying rating must be selected.
    • You can improve your design. Hope this helped.
    2
  • Raj Prasad Sanjel•980
    @RajSanjelPosted over 2 years ago

    Overall it looks great but you should do following things-:

    • That cyan color is for current day if you read the readme carefully.
    • On hovering the chart the opacity of bar should decrease a little bit(can be seen on hover effect design pic).
    • The problem with price while clicking in mobile should be fixed. Image of it going beside the bar click to see the image.
    • Don't keep unecessary comments in the code.
    Marked as helpful
    1
  • Kourosh Riahidehkordi•10
    @KooroshooSubmitted over 2 years ago

    What did I find difficult while building the project? I couldn't get rid of the small gap between the image and the bottom of the container

    Which areas of my code I am unsure of? The way I implemented the image swap when switching to mobile devices. Not sure if this is the correct way to do it.

    Do I have any questions about best practices? Any feedback about my implementation which can be improved upon is welcomed.

    HTML, CSS

    1
    Raj Prasad Sanjel•980
    @RajSanjelPosted over 2 years ago
    h1, h2, h3, h4, h5, h6, p, blockquote, pre,
a, abbr, acronym, address, big, cite, code,
del, dfn, em, img, ins, kbd, q, s, samp,
small, strike, strong, sub, sup, tt, var,
b, u, i, center,
dl, dt, dd, ol, ul, li,
fieldset, form, label, legend,
table, caption, tbody, tfoot, thead, tr, th, td,
article, aside, canvas, details, embed, 
figure, figcaption, footer, header, hgroup, 
menu, nav, output, ruby, section, summary,
time, mark, audio, video {
	margin: 0;
	padding: 0;
	border: 0;
	font-size: 100%;
	font: inherit;
	vertical-align: baseline;
}

    Instead of that big line make it

    *{
margin: 0;
   padding: 0;
   border: 0;
   font-size: 100%;
   font: inherit;
   vertical-align: baseline;
}
    • And that little space is because of line height in line 31 make it 0 and it will look as you expected. Hope this helped.
    Marked as helpful
    0
  • Raj Prasad Sanjel•980
    @RajSanjelPosted over 2 years ago

    My suggestions:-

    • Use semantic html (read here)[https://medium.com/before-semicolon/10-html-semantic-tags-and-when-to-use-them-5ae7d7d0b0f2]
    • According to challenge cyan is for current day so add logic for highlighting current day by using cyan color.
    • In mobile its upper part and lower part is touching the edge. You should fix that by giving margin top and bottom or padding top or bottom.

    Hope this helped.

    Marked as helpful
    1
  • karan karan•30
    @karanbadhwarSubmitted over 2 years ago

    Hello Guys, I hope you all are doing good. I made this app without React, and please do let me know What else could have I done with the CSS, because I struggle in CSS somewhat? I apologize in advance as I am a newbie here, if I missed something

    Interactive rating component

    1
    Raj Prasad Sanjel•980
    @RajSanjelPosted over 2 years ago

    Suggestions-:

    • Use semantic html read about it here.
    • You can center your design by
    body{
    min-width:100vw;
    min-height:100vh;
    display:grid;
    place-items:center;
}
    • There is a bug in the code. When user clicks on multiple rating and clicks submit it show all the number. If I click on 2 then 3 it says you have rated 23.
    Marked as helpful
    1
  • Haider Fadel•360
    @z3sbSubmitted over 2 years ago

    if you have any advice for me i will be happy

    #cssGrid

    1
    Raj Prasad Sanjel•980
    @RajSanjelPosted over 2 years ago

    Everything looks great but when there is error and user puts his/her email and gets out of the email box it shows email@example/com and user email can't be seen. You should fix it.

    0
  • Marcel Qayoom-Taylor•30
    @Marcelqayoomtaylor135Submitted over 2 years ago

    I found managing the margins difficult as adjusting them seemed to impact other elements when I didn't intend for that to happen.

    Flexbox. While it works, I'm unsure if grid would have been better here.

    How do you manage margins in the best way?

    Responsive landing page using SCSS

    1
    Raj Prasad Sanjel•980
    @RajSanjelPosted over 2 years ago

    Congrats on completing 1st challange:-

    • You can give padding instead of margin(margin is outside)(padding is inside).
    • Improve mobile version the design is touching the edge. (you can give margin-top and bottom).
    • Use semantic html read about it by clicking here

    Hope this helped.

    Marked as helpful
    1