Alan Mendoza• 100
@llalanmendozall
Submitted
Feedback pls
Rohit Purkait
@codeswithroh
All comments
- Rohit Purkait• 460
@codeswithroh
Posted
Everything looks fine bruv. Great work
Marked as helpful
0 - King David• 20
@ronalking182
Submitted
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
Rohit Purkait• 460@codeswithroh
Posted
In your github repo change the name of the index1.html file to index.html to be able to deploy the site properly.
0 - Sofia Skrbic• 125
@sofskrbic
Submitted
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.
Rohit Purkait• 460@codeswithroh
Posted
You 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 -
- joni• 365
@joni475
Submitted
Rohit Purkait• 460@codeswithroh
Posted
For some reason your site and repo is not visible. Maybe you have given the wrong links. Kindly look into that matter.
0 - Héctor Vásquez• 510
@Heitoluis
Submitted
Your feedback will be apreciated! =)
Rohit Purkait• 460@codeswithroh
Posted
You 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 - Carlos Galeano• 50
@shockiu
Submitted
Someone could give me advice, about my solution, this is my second challenge completed :)
Rohit Purkait• 460@codeswithroh
Posted
You 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 -
- RishkaA• 70
@RishkaA
Submitted
That's my second project here. Any comments on improving code are always welcome :)
Rohit Purkait• 460@codeswithroh
Posted
Congrats 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 -
- Rohit Purkait• 460
@codeswithroh
Submitted
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 😊
Rohit Purkait• 460@codeswithroh
Posted
I didn't checked the accessiblity issues. I should have checked them before submission
0 - Nono• 20
@NonoBtw
Submitted
Give me a feedback if you have advices :) I'll really appreciate ^^
Rohit Purkait• 460@codeswithroh
Posted
You 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 -
- Zak Croft• 50
@zakcroft
Submitted
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.
Rohit Purkait• 460@codeswithroh
Posted
You 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 - Borubar• 115
@Borub-ar
Submitted
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!
Rohit Purkait• 460@codeswithroh
Posted
I 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 @bogdiusf
Submitted
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!
Rohit Purkait• 460@codeswithroh
Posted
To 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- Estèlle• 40
@meijerestelle
Submitted
Hi! I used this challenge to return back to learning coding after a period of not doing it.
Some things I had trouble with, but would like some feedback on:
- The rounded corners thing. Couldn't get it to work on the container, so I put them on the individual cards and played with negative margins. Must be a better way to do this I think? Any suggestions?
- The rounded corners - but vertical. Due to the margins applied on the desktop view, the two bottom cards where slightly shifted to the left. Tried to overrule this with @media, but didn't really work. So I put negative margins on all cards and compensated it with 10px padding in the @media.
Thanks in advance for feedback!
Rohit Purkait• 460@codeswithroh
Posted
Instead of using border-radius just use border-top-left-radius or border-top-right-radius. The same follows for the bottom radius as well. This will help you to get the rounded corners in the specific regions as required. For more clarity you can check out my solution as well (https://github.com/codeswithroh/Simple-Card-Layout)
0 - shiva santosh jana• 375
@santu369
Submitted
Hi Mentors,
Coded the component using desktop first approach. Any code, responsive layout suggestions are welcome.
Rohit Purkait• 460@codeswithroh
Posted
You did an awesome job. In fact, I learned a new thing from your code ( the mix-blend-mode property). I created the same effect by putting a background color and setting the opacity of the img to 0.5 but your process looks better to me.
0 - Tracie• 40
@sweetemulsion
Submitted
Hello!
This is my first project here, I am still learning front end development and could use some tips on how to make my responsive coding a bit better :)
also, the images aren't showing on github for some reason....and I'm just realising I was sizing everything on my large monitor. I can move that but the responsive query still remains
Rohit Purkait• 460@codeswithroh
Posted
Congratulations on completing your first challenge. For your queries,
-
For making the site responsive make sure to use flexbox or grid to make the whole layout instead of positioning each element separately using "position: relative"
-
The img source given in your site shows that your image lies within a folder called "3-card-preview" but in your github you have not put your images inside a folder so, that is why it is not showing. Try to remove the folder name from your <img> src or try adding a folder with the same name as above and put the images in it.
Happy Coding 😃
1 -
- Paul Lorence• 20
@prLorence
Submitted
This took me hours to complete because GitHub won't show the images but I fixed it by adding "../" to the start of my styles directory.
Also, this is my second challenge so any feedback would be much appreciated!
Rohit Purkait• 460@codeswithroh
Posted
You did an awesome work. But there are some areas of improvements like
-
Adding a max-width or margin to the h2 and p would have been nice
-
Adding hover effects to the buttons and links would look nice as well
0 -
- Iryna• 110
@Web-Designa
Submitted
I used flexbox to make it responsive. This is my first solution here, will be really glad to here your opinion, thanks.
Rohit Purkait• 460@codeswithroh
Posted
You did an awesome work. But there are still some room for improvement regarding this solution like
- From the beginning of 970px the card-component sticks to the side of the screen
The problem is caused by the margin in card-component class. You have assigned margin in three sides only. Just change it to "margin: 140px 100px" and that will solve your problem.
0 - wisniewskiz• 55
@wisniewskiz
Submitted
Any feedback would be great! Sometimes I feel that I'm not sure the best way to go about positioning elements with padding and margins or with absolute and relative positioning, etc.
Rohit Purkait• 460@codeswithroh
Posted
First of all appearance-wise, bigger fonts and line-height would be a little better. Next about the coding part, I think that instead of setting a width in % for every component just write the html and give some padding and margin to it and instead of writing media query for each element separately try to write it in one place but its just my opinion, there's no harm in your style. For positioning I would tell you to follow the process I mentioned above because I personally don't like absolute or relative positioning because that makes the responsiveness a bit hard. But at the end you did a great work
1 - Mara García• 20
@mara-garcia
Submitted
Hello everyone!
I really need some feedback in general with the HTML file as well as the CSS. Also, I did have some trouble with the HTML background-image and I didn't really know how to make it better.
Thank you!
Cheers!
Rohit Purkait• 460@codeswithroh
Posted
Well setting up the background image with vh and vw is a little tricky but when you get the hang of it then it becomes very easy. You actually did a pretty great job in this project. With my current knowledge I have no advice to give to you regarding this project. Just keep coding 😃
0 - Pawel G Pasich• 30
@pawelgp
Submitted
Is this the right way to use BEM? Is there a way to colorize fonts with a background colour of a parent element other than just assign the same colour to the color property?
Rohit Purkait• 460@codeswithroh
Posted
The page you have made is nice but in the mobile version you should put some margins to keep the components from spilling over. Also, the max-width: 100% could be reduced to achieve the same effect as margins.
But overall, its great.
1 - Corvida Raven• 570
@SheGeeks
Submitted
Updated my solution on 6/29/21 to incorporate feedback from comments and a few new tricks I've learned since I originally submitted this solution.
Leave a comment below with a rating of 1-10 on my design or code (10 being the highest compliment).
Rohit Purkait• 460@codeswithroh
Posted
There are a lot of things that needs to be addressed here.
-
You have not created a nav for the desktop version
-
Your main content is not bound by flex or grid. That is why it fails the cross-browser test.
-
Don't use large pixel values instead of that use rem or em, its much more responsive
-
The positioning of almost all your elements is done by margin. Although that is good but when the components are in flexbox or grid. Positioning the element with only margin will mess up the positions across different screens.
-
You should add a hover effect on the nav-links. Although its my personal opinion.
One of the article that helped me with responsiveness is https://learn.shayhowe.com/advanced-html-css/responsive-web-design/
Hope, you will learn something from it as well.
Happy coding :-)
2 -
- imjackfrost1997• 145
@imjackfrost1997
Submitted
Any feedback will be a big help thanks :D
Rohit Purkait• 460@codeswithroh
Posted
This is really made nicely. The little animation onload is good. I am really impressed.
1 - Esteban Guzman• 20
@Elridge2
Submitted
Hello everybody, just completed this design using bootstrap. I think I managed to get it pretty accurate on desktop but my scaling/responsiveness is a bit off on mobile, could anyone suggest any tips on how to improve that? Thank you!
Rohit Purkait• 460@codeswithroh
Posted
I don't use bootstrap but on inspecting the site I figured out the problem is caused by width:100% which is set on the whole component. So, you could decrease that width to 90% and that will make your site a lot better.
1 @krudX
Submitted
I tried to add border radius to the section but wasn't able to do it. I'm open to suggestions.
Rohit Purkait• 460@codeswithroh
Posted
Since, this is a single layout so, try to use max-height and max-width to restrict it from over-flowing. And for the border-radius part. You first need to add a border element and then only you can use the border-radius.
0
