@GProostSubmitted about 1 month ago
What are you most proud of, and what would you do differently next time?
For some reason, the image is not staying inside of the border.. Not really sure on how to fix this...
Jani-B
@Jani-BAll comments
- @Jani-BPosted about 1 month ago
Hello,
Overall looks nice but as you already told you had some issue with the img.
Try to change the img "min-width: 100%" to just "width: 100%" Then it will take the 100% of the space available inside the container (width).
I hope this helps :)
Marked as helpful0 - @SurajCaseySubmitted about 1 month agoWhat are you most proud of, and what would you do differently next time?
I am proud that the webpage is responsive to different screen sizes. I would use both grid and flexbox to do it differently next time.
What challenges did you encounter, and how did you overcome them?There were two different images for mobiles and desktop screens. I did not know how to replace for different screen sizes. But while doing this project I learned to replace the images by hiding image and showing only the related image.
What specific areas of your project would you like help with?There is problem with as I have used it for the deducted price and last price. I used grid to show it in the same line. So, the placement for the mobile screen is not exact as the mobile design provided.
@Jani-BPosted about 1 month agoHello,
Looks very good :)
With the price and last price. I think the problem you mentioned comes from the following: You have named the divs "price" and then the childredn divs "recent price" and "deducted price". The problem is that when you give the "price"-div a display: grid -> it will also give it to the children as they also have the div with "price" class. I would name the children as "recent-price" and deducted-price" so you would not run to problems so easily. Then it would not copy the class "price" display: grid to all of the divs.
I hope this helps. Great work with the code :)
Marked as helpful1 - @isabellapabonsSubmitted about 2 months agoWhat specific areas of your project would you like help with?
I had to use a negative margin.
(margin-top: -23rem) on .jeanette-container and .patrick-container
I tried grid-gap and row-gap instead but it did not work. Any ideas on how to reduce the large gap between the grid items and reduce the height of the containers ?
@Jani-BPosted about 2 months agoHello,
I would say it is nice work in all :)
I think the problem here is the .kira-container. When the display is over 1400px you have to add to the kira-container the grid-row: 1 / span 2. Currently it takes only 1 column and 1 row. However as the container is longer than the others it will push the 2 other containers long way down. So if you use the grid-row 1 / span 2 -> it should fix the problem. Then you do not need to use the - 23 margin. Then you can use the gap.
Then you can also probably remove the max height 35% from the jonathan and daniel containers (when over 1400px screen width). Then I would also remove the body-containers margin as you can use the gap: 2 rem or similar instead.
It might create a problem on the smaller screen sizes if you remove the margin completely. To fix this I would change so that you use Grid on all the different media query versions and on the wider screens just change the grid-template to different.
I hope you this helps :) sorry if my writing is not the best.
Marked as helpful0 - @RichardTagoeSubmitted about 2 months agoWhat are you most proud of, and what would you do differently next time?
This time I was able to make my webpage a little more responsive. I was also able to learn how to use Dev tools to help you make your webpage more responsible. Next time i will try to make my CSS code a little more consice.
What challenges did you encounter, and how did you overcome them?When I finished the work, I found out that my work when displayed on my own laptop was very different from what I was anticipating because I didn't use my screen but rather the chrome dev tools through out the work. i don't know if what I did is correct, though.
What specific areas of your project would you like help with?I would like if someone is able to check if my work is responsive and explain to me why my work appeared different on my desktop from what I used
@Jani-BPosted about 2 months agoHello,
From my opinion the responsiveness works just fine. and the design looks good. Great work :)
I am not 100% sure why you used so huge fonts on this one. but the design itself and the responsiveness were good. Also you were wondering why your work looked different on your desktop. Maybe you had zoomed on the page? I noticed that the page looks a lot different if you zoom the page - so maybe that is the reason?
Just a hint for the fonts and other sizes -> you can also check our how to use rem instead on px. it works in many situations better.
Only thing i personally (just my personal opinion) would change is the inline css in the index.html. I think it is fine as it is but if you would create a bigger project and you use .css file and also add css in the HTML file - it might get confusing if you get back to the project later and want to change some css.
But I think you code was very clean and easy to follow. ! nice one :)
Marked as helpful0 - @miomio123Submitted 3 months agoWhat are you most proud of, and what would you do differently next time?
I think result is quite close to the original design
What challenges did you encounter, and how did you overcome them?Took me a long time to understand the grid responsivness
What specific areas of your project would you like help with?Im not sure if my code is clean enough, is there any other ways how to create it ina more efficiant way
@Jani-BPosted 3 months agoNice work. I like the hover effect that you added on the cards.
For the grid I would say it might be better to do it with: grid-template: card 1 for the column 1 + row 1 to 2, card 2 for the column 2 + row 1 card 3 for column 2 + row 2 card 4 for column 3 + row 1 to 2
Sorry Could not write it as grid-template should as the reply field made changes to the order of the words :D I hope you understand what I try to say :D
so that you do not have to use the margin -140px or similar. You did very well with it as the solution works well on the mobile and desktop, but I think the grid-template area would work more easier.
just give the card1 and card4 margin auto and they should sit on the middle and then card 2 and card 3 just fits top of each other.
But I would say the solution was very good :)
Marked as helpful1 - @H-KeshkSubmitted 3 months agoWhat are you most proud of, and what would you do differently next time?
*Reaching such a result after completing both HTML & CSS in around 4 months. *Being able to use responsive layout techniques correctly *Trying to finish the project with the minimum number of classes, using the semantic tags names as selectors.
-- Next time I will try to minimize the codes' lines, and write down the media queries codes in more prettier and shortened yet effective way.
What challenges did you encounter, and how did you overcome them?*Add colorful (hr) between sections. -- I found the answer on StackOverflow website, which was adding a top border with the color you want like in the following code: hr { border-top: 1px solid hsl(30, 54%, 90%); }
*How to use internal fonts after using to add fonts by it's external links. -- First mentioning the font from you project's folder:
Then adding that code in CSS: @font-face { font-family: 'Outfit'; src: url(assets/fonts/outfit/Outfit-VariableFont_wght.ttf); }
*What are the standard dimensions I'm going to use for media queries as screen sizes.. -- I found the solution on Bootstrap website under the section of (Breakpoints) giving the latest dimensions of all usable screens, up to date..
What specific areas of your project would you like help with?I'd like always help with making my code looks more simple and short. Also I'd like to have more help with media queries and moving from query to another smoothly..
@Jani-BPosted 3 months agoLooks good.
I like the good commenting on the CSS. That is something I very often forget to do. It looked very good.
- I noticed that you forgot to change the width 55% for the media query 576px version. for the extra small just make the width something like 100% and everything should work just fine.
It will also help you if you make the general layout mobile first. -> start doing the css for the mobile size. and then change it with media query for bigger screens. This way there is lot less problems as it is easier to adapt to the growing screen size than to screen size that gets smaller.
I hope this helps :) Your solution is very nice !
Marked as helpful1 - @DavidmercySubmitted 3 months agoWhat challenges did you encounter, and how did you overcome them?
I am a beginner. I found it challenging to add a white background to the right part of the design while the background color is different. Getting the accurate shade of green is also challenging.
What specific areas of your project would you like help with?I would be very grateful to receive comments on what I should have done differently so that my next projects will be much better.
@Jani-BPosted 3 months agoHi,
The solution is very close to the given design. There is few things that might help you:
- If you would wrap the picture and text divs in same "card" container it would help.
- What i did myself -> i made the container flex like you also did. But i also added div class "card" and made that a grid with 2 sides. So that way you can control the "card" more accurately. Now when I change the display size it will brake because the picture container and text container have their own lives.
- Also if you add some @media queries that will take effect for example in 800px width and other one that will take effect in around 400px -> you can change font size, padding etc to make sure the text will not brake your design on very small screen widths.
- Also very important addition. Use the flexbox to justify-content: center and align-content: center to get the items to center the screen. (remember to use height: 100vh for the flex container. -> it will make the height of the container 100% of the screen width so that the item really is centered on the screen).
I hope these comments help you.
Good work with the design :)
1 - @sudhanshusingh-gSubmitted 3 months ago@Jani-BPosted 3 months ago
Looks good :)
Unfortunately the buttons did not work atleast on my browser. Only the first click did work but I could not remove the changed div with second click. Maybe one solution could be that you use javascript to add class to the existing div. You can for example create a CSS class that has display: none; and then just use Javascript on click to give the specific div that className so that it will hide or display the div.
Then it is easy to use if statement to check if the div is showing or not and then remove or add the class accordingly
Sorry if my reply did not make sense :D Had some problems to find the correct words for this :D
The design itself looks very good. Nice work on the CSS.
0 - @Ibtehaj-Ali-1Submitted 4 months ago@Jani-BPosted 4 months ago
Very nice!
The design looks very good and the extra media query also made sure that everything works correctly on different screen sizes. Your code is also very good looking.
Great work !
1 - @GamingClausSubmitted 4 months agoWhat are you most proud of, and what would you do differently next time?
i am proud of the whole thing that i have done because this is my first type trying out the challenge and next time i would want to do it better
What challenges did you encounter, and how did you overcome them?while doing this challenge I faced a lot of difficulties like doing the mobile version layout as well as setting up the image that covers the div in the mobile layout and I still think its imperfect and can be done better but i am still proud of it
What specific areas of your project would you like help with?i would like a help with the mobile version specifically because of the problems i encountered while doing it also i was new to this whole thing.
@Jani-BPosted 4 months agoHello,
This looks good :) You were saying that you had some issues with the mobile version image that it does not cover completely the div. I noticed that you had 5px margin on the img. If you remove that on the mobile then it should be fine.
I noticed that on the around 425px size (mobile) the page breaks a little because the nutrition table element. You can change the padding for that and the problem should go away. You can add second media query with different with to fix this kind of problems if you want.
I have also been struggling a lot with my own pages going wrong on the mobile version. One very good way to avoid this is to make the site first mobile size and the with the media queries make it fit the bigger screens.
From my opinion your code and site looks very good.
Marked as helpful0 - @andmccutchanSubmitted 4 months agoWhat are you most proud of, and what would you do differently next time?
How similar/clean I was able to get it to the original design
What challenges did you encounter, and how did you overcome them?Determining spacing etc. I have not done too much stuff messing with line-spacing and kerning to that was new
What specific areas of your project would you like help with?For all the lists in the project, I could not figure out how to get new lines to stay indented with the previous lines while keeping the bullets inside of the element
@Jani-BPosted 4 months agoLooks very nice :)
You were asking about the list items and the numbers and bullets. Do you need to use the list-style-position inside? I think that has the major effect on how the text is shown. I think you should keep the bullets and numbers outside so they will not effect the text flow. (if you decide to change it - remember to remove also the li::before margin - so it will not effect your design.)
But from my opinion the design and code looks very clear and I like it :)
0 - @AtatraSubmitted 4 months agoWhat are you most proud of, and what would you do differently next time?
So, I'm finally starting my webdev journey. What I'm not proud of is that it took me way too long to achieve this challenge, more or less. But I'm happy that I took enough time to do research, and try to really grasp what I was doing. Yeah I know.. using NextJS is probably overkill here, but I want to learn how to use this framework, so I'm glad I did!
What challenges did you encounter, and how did you overcome them?It's my first time doing responsive design, and my first attempt at really trying to organize as much as possible my html page, so I had to google a lot of stuff.
What specific areas of your project would you like help with?It's a bit unfortunate that I couldn't make the page look right with super low width ( tag... Also, due to me flex-wrapping words, the text is unreadable at such dimensions. Finally the article gets resized and the beige background becomes visible again. If anyone has any suggestions, I would be very thankful!
@Jani-BPosted 4 months agoLooks very nice. (i did not find the white space).
Code and page both look very good and I think it is not a bad idea to use Next.js with a simple page because it is good training. I do it also for small projects to get good experience. About the super low width. i noticed problem only well under 300px but I think that is not something that people see often. The nutrition table at the bottom is the cause for that. smaller font size and smaller padding will help there, but I think it is not a real issue.
Very good looking page :)
Marked as helpful1
