Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

All comments

  • kirty 380

    @kirtymeena

    Posted

    Hi,

    The UI looks good. Great job 👍.

    But it is not responsive. For a small screen, the size doesn't readjust. I would suggest you learn about media queries.

    Also, the card seems to touch the edges of the screen from the top and bottom so you can add some padding for the top and bottom.

    1
  • kirty 380

    @kirtymeena

    Posted

    Hi soheil ramezani

    This was a tough challenge, but you did a great job. On the destination, crew, and technology page, the background image is not covering the entire page. Try removing the CSS in the class destination.

    .destination {
      height: 100%;
    } 
    

    For crew as well you can remove the below class from the CSS file and it will work fine.

    .crew{
        height: 100%;
    }
    

    There are some accessibility errors. to remove it you can add an alt text in img. tag

    0
  • @astijusk101

    Submitted

    How could I make the mobile layout better? In the design file given, it seems as if there is more space between the edge of the window and the NFT card, however in my project there isn't. I don't know how to add it.

    kirty 380

    @kirtymeena

    Posted

    Hi,

    The UI part looks nice but it's not responsive, you need to add media queries (mobile first or desktop first approach) and use min-width or max-width. You can read more about mobile first and desktop first here

    To add space on the left and right sides, inside media query for small screen add

    body { 
          padding-inline:2rem;
    }
    

    Keep coding.

    0
  • kirty 380

    @kirtymeena

    Posted

    Hi @Nexuuuus

    You can use the line-height property to decrease or increase the gaps between the lines. You can read more about it here

    2
  • kirty 380

    @kirtymeena

    Posted

    Hi, I have noticed some issues like

    • when navigating to technology your image is on top of the navbar.
    • your image in the destination is not displayed properly.

    Marked as helpful

    0
  • @TotallySly

    Submitted

    I made an error where initially I was using buttons instead of radio buttons. This meant that I was finding it impossible to create the greyed out 'active' state with the ratings. However, I received some help and it worked and made life so much easier.

    I was wondering if using a string template literal would be possible. I tried but I could not quite figure it out.

    Also, my JS are not within functions. Is this bad practice? Should I be saving the script within functions?.

    Thanks!

    kirty 380

    @kirtymeena

    Posted

    Hi Dizzy_Sloth,

    • On lines 37 and 41 in the index.html file there are open and closing brackets, I am not sure why they're there but I can see them in the UI and according to the design there are no such brackets in UI.

    • Users should not be able to submit if they haven't selected a rating.

    Overall, it looks great. Keep coding 👍.

    0
  • @AditiChandra18

    Submitted

    The front-end for desktop layout was manageable to design but I am still not sure how to create it for mobile through the same code. Will really like some help with that!!

    kirty 380

    @kirtymeena

    Posted

    Hi Aditi, Good work with this challenge

    Your div is not in the center, You can use flexbox to center the div

    you can read about flexbox here https://css-tricks.com/snippets/css/a-guide-to-flexbox/

    you can try this extension called PerfectPixel it will help you create your design pixel perfect.

    Keep coding 👍.

    Marked as helpful

    0
  • @Tessie475

    Submitted

    Anyone with a different approach of aligning div to center, I'd appreciate a reply from you. General feedback is welcome too.

    kirty 380

    @kirtymeena

    Posted

    Hi, you did a great job with this one, just a few issues

    1. the layout is touching the right and left boundary for small screens. You can write media queries for small screens.

    2. you can use a flex-box to center a div.

    ` .row { min-height: 100vh;

        display: flex;
    
        justify-content: center;
    
        align-items: center;
    }
    

    `

    You can read about flexbox here https://css-tricks.com/snippets/css/a-guide-to-flexbox/

    keep coding 👍.

    0
  • kirty 380

    @kirtymeena

    Posted

    Hi, Nice work the design looks good but there are a few issues I have noticed:

    1. your container is not at the center of the page. You can add below CSS code in the body min-height:100vh; display: flex; flex-direction: column; justify-content: center; align-items: center;

    2. You haven't implemented the active states like on hover, buttons don't change color also, you should add cursor: pointer to the buttons in CSS. you can also add an active selector to the buttons so that it will look good when clicked.

    apart from few issues, you did a great job.

    keep coding 👍.

    0
  • Nika 180

    @nikavolk

    Submitted

    // EDIT: I've implemented kirty's suggestions (in the comments) and centered the Thank you text, as well as added code that disables the submit button until a rating is chosen. Thank you kirty! ❤️

    Hi!

    This is my very first, ever, project that I've done fully on my own.

    I've used ReactJS and Sass to build this component.

    I definitely think I didn't use Sass to the fullest, however it was my first time using it, so I've used variables and nesting. Any feedback in regards to making the css file smaller are welcome.

    I am also aware that I could have split the CSS styles for each component, however having looked into that, I would have to work with importing them, and I feel that this project is small enough it didn't warrant splitting the files up.

    kirty 380

    @kirtymeena

    Posted

    Hi, nice work completing this challenge. I have noticed few issues:

    1. Thank you text is not in the center.
    2. user should not be able to submit without selecting a rating, I am able to submit which I think is not correct. You can create a form and use onSubmit to check if the user has selected any number or not.

    if you add the flip card effect on submitting it will look even better. Adding an active selector to a button makes it look nice when clicked on it.

    apart from small issues, the design looks good just thank you text and text below it are not centered.

    Keep coding.

    Marked as helpful

    3
  • @dostonnabotov

    Submitted

    Hi, everyone. I just completed this challenge. It went quite well, though. I would be grateful if you could give some feedback or show where I made mistakes.

    kirty 380

    @kirtymeena

    Posted

    Hi, It looks excellent similar to the design, but adding an active selector to card links will look nice when those links are pressed.

    Keep Coding. 👍

    Marked as helpful

    1
  • Elodie 50

    @elodiehg

    Submitted

    I'm not sure about how to display the hero's image into my div card. Any feedback is welcome !

    kirty 380

    @kirtymeena

    Posted

    Hi @ElodieHalgand,

    Use semantic HTML and use alt in the image tag. In img tag use "/" rather than backslash. doing so will remove the HTML issues and Accessibility Issues. something like this : <img src="images/illustration-hero.svg" id="hero" alt="hero" >

    Overall it looks good.

    keep Coding 👍

    Marked as helpful

    0