Any tips or advice ? New to coding and I'm going the self-taught route so any advice helps.
kirty
@kirtymeenaAll comments
- @KinginGuzmanSubmitted over 1 year ago@kirtymeenaPosted over 1 year ago
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 - @soheilRamezani-DevSubmitted about 2 years ago@kirtymeenaPosted about 2 years ago
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 - @astijusk101Submitted over 2 years ago
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.
@kirtymeenaPosted over 2 years agoHi,
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 - @NexuuuusSubmitted over 2 years ago
How would one go about changing the gap between lines in the small text?
- @besttlookkSubmitted over 2 years ago
Feedbacks are always welcome,
Thanks,
Happy coding
@kirtymeenaPosted over 2 years agoHi, 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 helpful0 - @TotallySlySubmitted over 2 years ago
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!
@kirtymeenaPosted over 2 years agoHi 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 -
- @AditiChandra18Submitted over 2 years ago
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!!
@kirtymeenaPosted over 2 years agoHi 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 helpful0 - @Tessie475Submitted over 2 years ago
Anyone with a different approach of aligning div to center, I'd appreciate a reply from you. General feedback is welcome too.
@kirtymeenaPosted over 2 years agoHi, you did a great job with this one, just a few issues
-
the layout is touching the right and left boundary for small screens. You can write media queries for small screens.
-
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 -
- @ugochukwuuuSubmitted over 2 years ago@kirtymeenaPosted over 2 years ago
Hi, Nice work the design looks good but there are a few issues I have noticed:
-
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;
-
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 -
- @nikavolkSubmitted over 2 years ago
// 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.
@kirtymeenaPosted over 2 years agoHi, nice work completing this challenge. I have noticed few issues:
- Thank you text is not in the center.
- 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 helpful3 - @dostonnabotovSubmitted over 2 years ago
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.
@kirtymeenaPosted over 2 years agoHi, 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 helpful1 - @elodiehgSubmitted over 2 years ago
I'm not sure about how to display the hero's image into my div card. Any feedback is welcome !
@kirtymeenaPosted over 2 years agoHi @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 helpful0