P
Saulius Gražys• 210
@sgrazys
Submitted
I tried to do the button as API.
If you have any comments I would be happy to hear :)
Kamasah-Dickson
@Kamasah-Dickson
All comments
- Kamasah-Dickson• 5,610
@Kamasah-Dickson
Posted
Hey @sgrazys i checked your solution. it looks great and responsive. but on mobile devices the cards looks a bit small and there is so much white spaces on the left and right. try to increase the max-width of the cards. by the way great job.👏👏
0 - Brijeshkumar Patel• 200
@brijeshkumar001
Submitted
Hi I’m Brijeshkumar and this is my solution for this challenge.
Any suggestions on how I can improve and reduce unnecessary code are welcome!
Thank you. 😊
Kamasah-Dickson• 5,610@Kamasah-Dickson
Posted
Hello there, you are really doing a great there but these are a few things that I noticed .
- I realized your card is not responsive. use a max-width property instead of just width. the width property on the main behaves like a block element which may cause overflows sometimes.
- I noticed you are using floats to layout your card which makes it unresponsive. Avoid using floats. always use responsive layout systems like flexbox and grid.
- You missed some of the background properties like the background-position
I think you did a really great job.🔥 Happy coding🚀
0 - Scott Tabor• 150
@scottttabor
Submitted
I used Javascript to be able to change the source of the image instead of HTML srcset. I just wanted the practice of using javascript on a project! Id love any feedback on the format of my code, how I wrote my javascript code and included it in my HTML. My only question is that the product card is responsive in all the mobile options except when it goes below a width of 375px there seems to be some white space that shows below the main background. Any tips for why that is happening would be appreciated.
Kamasah-Dickson• 5,610@Kamasah-Dickson
Posted
@scott Tabor you did I really great job. your solution was very responsive as well. But one of the things you forgot to add was the type attribute for your button. some of the importance are.
- It helps to define the role and behavior of the button for users and assistive technologies.
- Providing a type for your buttons also helps improve the accessibility of your website.
- The type also determines if the button is a submit,reset or just a button.
I hope this was helpful. Happy coding🔥
Marked as helpful
1 - Kamasah-Dickson• 5,610
@Kamasah-Dickson
Submitted
Hello guys am finally here with another solution. This time I tried my hands with React and I think It is a better Framework so am staying with React for a while. I feel a bit intimidated with React but since am a beginner but feel free as always and show your opinion. Happy Coding :)
Kamasah-Dickson• 5,610@Kamasah-Dickson
Posted
Am finding it hard to fix the accessibility issues with react...should I run npm run dev or what. any idea??
0 - LorenaFrias• 80
@LorenaFrias
Submitted
This was more challenging than I thought it would be. I don't fully understand grid yet. Working on it. I always end up feeling my code could've been cleaner. Also, first attempt to implement BEM. I haven't grasped it yet. :) Feel free to comment!
Kamasah-Dickson• 5,610@Kamasah-Dickson
Posted
Your solution looks great and to help you with the accessibility issue 1.There should always be an h1 in your projects. It should be the first head tag you use. And should reduce in descending order. h1....h6
Happy coding🎉
1 - Konjeti Maruthi• 70
@MaruthiKo
Submitted
- Which areas of this project can be changed or reviewed?
- Any better approach to my existing solution?Glad to hear them
Kamasah-Dickson• 5,610@Kamasah-Dickson
Posted
Your solution looks great but i want to help you with some small tips.
1 . Do not specify any height in the grid items. This sometimes breaks layout and might stress you some other time when things are not working as expected. Instead allow the items to determine their own height. What i mean is that paddings and margins applied to the grid items will automatically determine the heights of the grid items. I hope you get me.
2 . To help you with the accessibility issues, remove the attribution class and change the div to a footer
Do not use a section tag if there won't be any direct head tag I mean h1....h6 always use a section tag if there would be a directed head tag in your page otherwise use a div instead.
Good job there👍🎉 Happy coding
Marked as helpful
1 - darryncodes• 6,430
@darryncodes
Submitted
Hi everyone 👋
A fun project to get back in the saddle after taking the summer out to avoid burnout. I wanted to try Tailwind CSS to see what all the utility-first CSS hype is about. Very intrigued to see how it might scale in a larger React app.
I added a mobile navigation and some JS client-side form validation.
Any feedback that could help me to improve would be very welcome!
Happy coding 🤙
Kamasah-Dickson• 5,610@Kamasah-Dickson
Posted
hi there🖐 your solution looks very great and responsive but one thing is that the background-color of the feature section is not set. it should be different from the actual body background-color which I know you noticed too. But also one thing again is that the svg is just above the feature section...kinda before. it just blends with the background-color of the feature section. besides, Great job there👍
Happy coding🔥🔥
1 - PKellen Ruyle• 930
@rule-kells
Submitted
Feedback is welcomed!
Kamasah-Dickson• 5,610@Kamasah-Dickson
Posted
But next time your close button should be in the same div of the nav-links
E.g
`` 1. <button>btn</button>
- <li> links</li>
- <li> links</li>
- <li> links</li> ``
This prevent the close button from scrolling outside of view.
besides you did really great👍
0 - Kirill Mishin• 280
@TheN0rt
Submitted
Project without using localStorage and dynamic time of comments
Kamasah-Dickson• 5,610@Kamasah-Dickson
Posted
Hi there👍 you did great on your solution but i suggest.
-
You use a max-width on the parent container of about 700px...you decide. The widths are spanning the whole screen that is why you need a max-width.
-
Use margin-inline:auto or margin:0 auto to center it in case you use the max-width.
-
To help with the accessibility issue, the should be a level one heading which is an h1 in your solution.
-
Also there should be a semantic tag in your solution such a main, aside and section. In your case I suggest you use the main to wrap your solution or change the div of the parent element into main
Good job and happy coding👍
1 -
@lichtle
Submitted
This is my second attempt on this challenge. This time I used the grid display (instead of flexbox) and improved the responsive aspect. If you want to check my previous solution, it's on my GitHub portfolio. Thanks!
Kamasah-Dickson• 5,610@Kamasah-Dickson
Posted
Hi there👍..You really did a very great job. And I like the fact that you came up with the solution in your own style with the colors and everything.....also the fact that you were able to decide on the best layout tool for this solution which is grid. It takes people a lot of time to know or notice the best layout tool for their solution. Since this is your second attempt I know for sure, you now have an idea about the best layout tool to use at the appropriate place.
Happy coding 👍
1- PKellen Ruyle• 930
@rule-kells
Submitted
Feedback is welcomed!
Kamasah-Dickson• 5,610@Kamasah-Dickson
Posted
Hi there👍...I think you really did a great Job there..keep it up.. 👍
- On the navigation on mobile, the height is not covering the entire vertical screen on mobile when opened. always use 100vh which is 100% a devices viewport height instead of just using height:100%.
Besides you did a very great job.👍 Happy Coding👍
0 - faizan alam• 30
@faizan619
Submitted
do say me how is my design I have not worked for responsive now..will try to implement next time.
Kamasah-Dickson• 5,610@Kamasah-Dickson
Posted
Hi there👍 you really did great. keep it up but these are some tips you can use next time.
-
Don't use units such as px and percentages for parent containers but instead I recommend rem units for font sizes and widths and em units for paddings. I don't recommend px units because they cause a lot of problems when people zoom in their browsers and there are a lot of reasons. You can learn more about units at YouTube. Check Kevin Powell and coder coder
-
Also I would really recommend that you take your time and learn flexbox and grid into more details don't rush take your time and believe in the process.
Besides you really did a very great job👍 Happy coding👍
Marked as helpful
2 -
- Syed Baasit• 110
@syedbaasit
Submitted
this was an amazing project looked easy at first but as a beginner it makes you think . i couldn't properly put a hover property on the "read me " text . if anyone can spot the irregularity comment it !
Kamasah-Dickson• 5,610@Kamasah-Dickson
Posted
Hi there👍...I really like the fact that you had an idea about how to come about with this project. But It is not responsive on mobile.
-
I recommend you start most of your solutions on mobile first and scale them to desktop by using media queries which I really know you have an idea about.
-
To help with the accessibility issues, please always use your headings (h1....h6) in descending order in your projects, don't mix them up.
-
There should always be at least one or two a semantic tags in your projects such as main, aside, header and footer there are a lot but at least these are the commonest.
I think you did a great job keep it up👍
Happy coding :)
Marked as helpful
0 -
- Abhishek katkam• 290
@Abiekatkam
Submitted
Happy coding!.
Kamasah-Dickson• 5,610@Kamasah-Dickson
Posted
Your solution looks great and you did a good job.
-
To help with the accessibility issue, your headings (h1.....h6) must be used in reducing order in your projects.
-
Do not use the section tag when there is not going to be any immediate headings use divs instead.
-
To clear the landmark issue, use the main tag for your main section/content. and use and aside for the side contents/news. besides I hope this was very helpful.
Happy coding and have a nice day.
Marked as helpful
1 -
- Toni Jokić• 550
@Tonicc00
Submitted
Kamasah-Dickson• 5,610@Kamasah-Dickson
Posted
You're doing pretty well but to help.
- Use a max width for your cards container and increase the width.
- Wrap your card in a main semantic tag to clear the accessibility issues. Besides you're doing pretty well.
Happy coding👍
1 - Solution Abiola• 30
@Soldiamond
Submitted
The desktop design went well, but I had issues with the mobile design as I tried to use media query.
I would love it if I would be put through in this. The design was fine for Desktop, iPad, but not for mobile. Corrections and guidance are welcomed
Kamasah-Dickson• 5,610@Kamasah-Dickson
Posted
To help with your accessibility issues
- The main is a semantic tag that describes a section of the page and it must be used once. So wrap the main tag around the project itself(the card).
- Don't use sections when there is not going to be any immediate headings(h1.....h6) use divs for sections that don't.
Hope this was helpful👍 Happy coding
0 - Solution Abiola• 30
@Soldiamond
Submitted
The desktop design went well, but I had issues with the mobile design as I tried to use media query.
I would love it if I would be put through in this. The design was fine for Desktop, iPad, but not for mobile. Corrections and guidance are welcomed
Kamasah-Dickson• 5,610@Kamasah-Dickson
Posted
Hey there....you're doing pretty great it's just that next time approach your projects on mobile first and and use min-width media query to scale them to desktop. But first remember to look at the layout pretty well before you start with mobile first else you're going to be too hard on your self. Also always use a max-width on your containers, this will allow most of your contents to have enough breathing room. Hope this was helpful. Have a nice day and a Happy coding
0 - Kml_joshi• 560
@KamalJoshi-web
Submitted
This is my 8th Frontend Mentor project and my first Intermediate where I am learned about cart functionality. And this challenge has tested my JavaScript skills. If I made any mistakes in this project, please feel free to correct me.
Kamasah-Dickson• 5,610@Kamasah-Dickson
Posted
- Your solution looks great but not responsive. Use max-width for your card instead of just width .
- To help with the accessibility issues wrap your card in a main semantic tag.
- Bonus Headings must be used in reducing order e.g h1,h2,h3,h4 do not mixed them up they should be used in descending order.
- Disable the next and prev button when there is a modal
- Add a margin between your header and the grid section. its too closer to the header and there is not enough space there.
- There should be a heading before a <section> tag. Don't use a section tag if there won't be any immediate heading.
I hope this was helpful.
Happy coding and have a nice day👍
Marked as helpful
0 - Abhijit Shankhdhar• 320
@im-abhijit
Submitted
hi all , please review my code and any feedback will be appreciated
Kamasah-Dickson• 5,610@Kamasah-Dickson
Posted
- Your solution looks great but not responsive. Use max-width for your card instead of just width .
- Don't use px for card widths, use rem to achieve responsiveness.
- To help with the accessibility issues wrap your card in a main semantic tag.
- Bonus Headings must be used in reducing order e.g h1,h2,h3,h4 do not mixed them up they should be used in descending order. I hope this was helpful😀
Happy coding and have a nice day👍
Marked as helpful
0 - Tivere IDORO• 60
@tivereidoro
Submitted
Do you think animations would make this page look better.?
Kamasah-Dickson• 5,610@Kamasah-Dickson
Posted
Your solution looks great👍
- Don't use px for card widths, use rem and em for paddings to achieve better accessibility.
- To help with the accessibility issues wrap your card in a main semantic tag.
- Bonus Headings must be used in reducing order e.g h1,h2,h3,h4 do not mixed them up they should be used in descending order.
- Do not use Px. I hope this was helpful.
Happy coding and have a nice day👍
0 @Erkhembayar-Atarbold
Submitted
What can I do next to improve my FE skills? Are my class names understandable? What did you find difficult while building? Which areas of your code are you unsure? Do you have any questions about best practices?
Kamasah-Dickson• 5,610@Kamasah-Dickson
Posted
- Your solution looks great but not responsive. Use max-width for your card instead of just width .
- Don't use px for card widths, use rem to achieve responsiveness.
- To help with the accessibility issues wrap your card in a main semantic tag.
- Your footer must be text align center.
- Bonus: Headings must be used in reducing order e.g h1,h2,h3,h4 do not mixed them up they should be used in descending order.
- Your p should be text align center. You are doing great man👍
Happy coding and have a nice day👍
0- CO• 80
@melco10
Submitted
What did you find difficult while building the project?
- Building responsive especially the function warn of form.
- Trial and error of building function warn and also my logic. Which areas of your code are you unsure of?
- My CSS and JS, I'm pretty sure it can be better in styling and also the function logic of Active warn state js. and also I'm struggling of figuring it out on styling code and JavaScript function code, because of my knowledge on front-end are still lacking yet.
"Feedback is welcome" "If some of the code lines are something off, feel free to correct me."
Kamasah-Dickson• 5,610@Kamasah-Dickson
Posted
- Your solution looks great but not responsive. Use max-width for your card instead of just width .
- Don't use px for card widths, use rem to achieve responsiveness.
- To help with the accessibility issues wrap your card in a main semantic tag.
- Your footer must be text align center.
- Bonus : Headings must be used in reducing order e.g h1,h2,h3,h4 do not mixed them up they should be used in descending order.
- Stop using px units use rem and em besides you are doing pretty good man👍
Happy coding and have a nice day👍
Marked as helpful
0 @Baleke
Submitted
Hello guys whatever feedbacks is welcome here
Kamasah-Dickson• 5,610@Kamasah-Dickson
Posted
- Your solution looks great but not responsive. Use max-width for your card instead of just width .
- Don't use px for card widths, use rem to achieve responsiveness.
- To help with the accessibility issues wrap your card in a main semantic tag.
- Your footer must be text align center.
- Bonus Headings must be used in reducing order e.g h1,h2,h3,h4 do not mixed them up they should be used in descending order. I hope this was helpful😀
Happy coding and have a nice day👍
0- Gabriel Kyalo• 50
@gabrielkyalo
Submitted
All feedback is welcome.
Kamasah-Dickson• 5,610@Kamasah-Dickson
Posted
- Your solution looks great but not responsive. Use max-width for your card instead of just width .
- Don't use percentages for card widths, use rem to achieve responsiveness.
- To help with the accessibility issues wrap your card in a main semantic tag.
- Headings must be used in reducing order e.g h1,h2,h3,h4 do not mixed them up they should be used in descending order. I hope this was helpful😀
Happy coding and have a nice day👍
1
