@llalanmendozallSubmitted almost 3 years ago
Feedback pls
Rohit Purkait
@codeswithrohAll comments
- @codeswithrohPosted almost 3 years ago
Everything looks fine bruv. Great work
Marked as helpful0 - @ronalking182Submitted over 3 years ago
pls i would love to here feedback on how to make my code more cleaner and also a rating to help me improve as a neewbie
@codeswithrohPosted over 3 years agoIn your github repo change the name of the index1.html file to index.html to be able to deploy the site properly.
0 - @sofskrbicSubmitted over 3 years ago
Any feedback is appreciated! :)
The initial problem I've noticed is with resizing the window as the navigation menu sometimes opens by default on mobile view.
UPDATE: The initial problem is fixed and it was pointed out to me there was some horizontal overflow, so that's fixed as well. Additionally, I've implemented a breakpoint for tablets also due to too large icons.
@codeswithrohPosted over 3 years agoYou did a great job regarding this solutions. Well, there are a few things that needs to be addressed.
-
As you told there is that problem with your navbar which pops suddenly during the change in screen size
-
There is a horizontal scroll which is a bit annoying
-
I would suggest you not to set style inside the tags itself because it causes problems
-
Your site looks good for screen sizes beyond 1440px but most of the user will view this site in 1440px and for 1440px the icons are a bit bigger.
Except these you did an awesome job. So, keep coding 😊
1 -
- @joni475Submitted over 3 years ago@codeswithrohPosted over 3 years ago
For some reason your site and repo is not visible. Maybe you have given the wrong links. Kindly look into that matter.
0 - @HeitoluisSubmitted over 3 years ago
Your feedback will be apreciated! =)
@codeswithrohPosted over 3 years agoYou did an awesome job on this one. I tried to find mistake in your solution but I couldn't find any 😂. The responsivness is on point, the site scales up and down properly. So. a really goo job.
0 - @shockiuSubmitted over 3 years ago
Someone could give me advice, about my solution, this is my second challenge completed :)
@codeswithrohPosted over 3 years agoYou did an amazing job with this solution. Well, there are two things that need to be addressed.
-
The password section keeps throwing an error even if the number of characters is more than 8
-
You used way too many flex-box. You have used flexbox in places that doesn't need it. You see by default all the elements are places in a column. So, you don't have to use flex for each element and set it to flex-direction: column
Except this two things you site looks pretty amazing. So, keep up the good work and keep coding 😀
0 -
- @RishkaASubmitted over 3 years ago
That's my second project here. Any comments on improving code are always welcome :)
@codeswithrohPosted over 3 years agoCongrats on completing your second project 😊. You have done a really good job. But there are some little things which need to be improved.
-
There is no border-radius on the container for the curved sides
-
The site is not fully responsive except the two targeted screen i.e. 1440px and 375px.
Btw Keep coding and hope to see your next solution soon 😊
0 -
- @codeswithrohSubmitted over 3 years ago
I spent quite some time with this project and I am content with the result. I have done all the animations in pure css. But this can be done easily with modules. So, if you guys know any efficient module which could help in animation then please comment it 🙏
Have a great day guys 😊
@codeswithrohPosted over 3 years agoI didn't checked the accessiblity issues. I should have checked them before submission
0 - @NonoBtwSubmitted over 3 years ago
Give me a feedback if you have advices :) I'll really appreciate ^^
@codeswithrohPosted over 3 years agoYou have done a great work. But there are some things that could be improved:
-
Adding margins at the top and bottom of the whole layout for small screens. Because at small screens your layout sticks to the top and bottom
-
Reducing the padding of the links for small screens is also required. Because it is overflowing the container.
0 -
- @zakcroftSubmitted over 3 years ago
Would like some comments on how I used BEM or any other ways I could have used Flex better in places.
However, any feedback appreciated in any areas.
@codeswithrohPosted over 3 years agoYou did an amazing job on this one. I really liked it. The only thing that's bugging me is the little horizontal scroll in full-screen. To solve that just use
body { overflow:hidden; }Except that everything looks great.
0 - @Borub-arSubmitted over 3 years ago
Hello everyone, I would be very grateful if someone review my javascript and CSS. It's not very long and I really need some feedback on this one.
Thank you!
@codeswithrohPosted over 3 years agoI really liked your work. Everything looks neat and clean. Only one thing is left and that is to add a hover effect on the footer social-icons. Except that everything looks great.
0 - @bogdiusfSubmitted over 3 years ago
Seems I have faced pretty major problems with dynamic responsiveness (container relative to device viewport, and child elements relative to container/parents). If anyone would be so kind to give me a piece of advice, based on the code i wrote, on how to not hard-code these things with N @media queries, I'd very much appreciate that!
Thank you!
@codeswithrohPosted over 3 years agoTo place your whole container at the center of the screen just add
display: grid; place-items: center;in your body tag. It will save you from using position:absolute.
And for the responsiveness try setting a flex-basis with minmax which will help you a lot in responsiveness.
You can check out my solution as well (https://github.com/codeswithroh/stats-preview)
1
