Jonatan Samuelsson
@jonatan-samuelssonAll comments
- P@Dzik0Submitted 5 months agoP@jonatan-samuelssonPosted 3 days ago
Looks really good, nice job!
I'd like to hear your thoughts on hard coding the rating system using JS rather than using custom styled radiobuttons, what made you go for that approach?
0 - P@taceseptSubmitted 15 days agoP@jonatan-samuelssonPosted 5 days ago
Well done, good-looking solution, well-structured code.
Your content is rendered quite high up on the page, is that a conscius choice? Asking since it departs from the design.
Also, no matter what quiz topic I choose, I get the HTML quiz. I didn't have to review your code enough to pinpoint the error, but you should look over your jason-fetching and data managment functions.
Marked as helpful0 - @Bytehax21Submitted 18 days agoWhat are you most proud of, and what would you do differently next time?
Making projects using Figma is way easier and faster than eyeballing the design stuff. I think i made it pretty close to original so no doubts on my work as of now.
What challenges did you encounter, and how did you overcome them?I faced issues with applying CSS on Child elements like p tags as assigning ID to them and then writing CSS didn't work on them so, I did a workaround by assigning the child a class and then using css and the padding was different than mentioned in Figma design file
What specific areas of your project would you like help with?Is my CSS good enough or how can i improve it like in applying padding margins and where to assign class or ID etc.
P@jonatan-samuelssonPosted 18 days agoHey.
Looks good, well done. One thing I noticed is you're using
padding-top
to position the card. Another way to do it where it gets centered on the page no matter what is to give your main something like:display: flex; justify-content: center; align-items: center; min-height: 100svh;
For your issue with child p-elements, there are multiple solutions. First of all, i'd say that the first paragraph semantically should be a heading (say h2, h3 or such).
If you still wanna keep it as paragraphs, you can either do the id thing like so:
.text #para-1 { ... } .text #para-2 { ... }
...or, you could go for child selectors:
.text > p:first-child {...} .text > p:nth-child(2) {...} ... .text > p:last-child {...}
Finally, all of this can be nested to make the css more structured:
.text { (some styles for text perhaps) p:nth-child(x) { (some style) } p:nth-child(x) { (some other style) } }
Marked as helpful0 - P@HelewudSubmitted 27 days agoP@jonatan-samuelssonPosted 18 days ago
I really like your solution, code is super clean and easy to read.
Especially the JS is nicely commented and compartmentalized. I like the idea of using a class like you did, as it makes for better portability and reusability. My solution had a completely different approach, so it's nice to see all the various way to achieve the same functionality.
Marked as helpful0 - P@loki-pepeSubmitted 21 days agoP@jonatan-samuelssonPosted 19 days ago
Really nice job!
Your JS is fantastic, super clean and DRY. I really learned from this for future projects, and realized how I should have done this project if I did it again.
For mine, I had trouble getting the inputs responsive in size and ended up hacking it via contenteditable divs. I don't know what you did differently, but you made it work.
One small thing I noticed is that the totals do not reset when the reset button is clicked.
Marked as helpful0 - @Donald-cmd-opsSubmitted 21 days agoWhat are you most proud of, and what would you do differently next time?
looks pretty close to the image
What challenges did you encounter, and how did you overcome them?some minor difference between image and my developed html so spacing button size differences
What specific areas of your project would you like help with?how to improve code to improve closeness to screenshot
P@jonatan-samuelssonPosted 20 days agoLooks good!
I see your problem with the likeness, and I think the easiest solution would be to get rid of the
width: 40%
on the card. Either set it to some fixed width or usemin()
to restrict its growth.Also, the way you center the
.centered
class is a bit overly complicated. A better way to do it is to have a full-viewport container (could just be the main) withdisplay: grid
andplace-items: center
. That way, the parent element controls the position of the card, which improves reusability.Happy coding!
Marked as helpful0 - P@codejerooSubmitted 20 days agoWhat are you most proud of, and what would you do differently next time?
The fact that I was able to use and implement media queries. I'm not really that confident but it works.
What challenges did you encounter, and how did you overcome them?It was hard to make it responsive to mobile and tablet screen sizes. I somehow made it work using media queries.
What specific areas of your project would you like help with?I hope I'll be able to receive feedback on how I can make it more responsive.
P@jonatan-samuelssonPosted 20 days agoNice work, good responsiveness!
A few small tweaks would be:
-
to create some space above and below the component at tablet and desktop sizes. You could get this by having
display: flex
andalign-items: center
on the main (or whatever wrapper you have), as long as it´s also gotmin-height: 100svh
-
to get rid of the border-radius for the component card at mobile size. If you go to mobile size and scroll all the way down, you see a small bit of the body bg in the lower corners. Another way to solve the same issue would be to set
body {background-color: #fff;}
at mobile size -
keep the mobile layout a bit longer. It switches at a very low width at the moment.
Happy coding!
Marked as helpful0 -
- @Mubsic-1Submitted 20 days agoWhat specific areas of your project would you like help with?
I think i should've built the Nutrition section using Grid and not Flexbox! am i right?
P@jonatan-samuelssonPosted 20 days agoNice work!
To your specific question: Yes, I think grid is better in this case, just because it lets the parent control the layout rather than the children, which is more what flex does (generally speaking anyway).
When I did this challenge, I even ended up using the old-fashioned
<table>
for the nutrition-section! :-)Marked as helpful0 - @guardianprimeSubmitted about 2 months agoP@jonatan-samuelssonPosted 20 days ago
Hi!
Looks nice, well done. Handling JS promises and various arrays, HTMLCollections etc can be a real hassle.
A few points of feedback:
Style, layout
- The hover state for the elipsis icon doesn't show.
- The responsive sizing gets a bit wierd sometimes. I think your breakpoint for switching layouts might be a bit low, as the desktop layout looks strange at narrower sizes. Also, at the other end, having the cards expand forever with screen size makes them awkward when the screen is really wide. I'd siggest using either fixed rem widths or
clamp()
/min()
to restrict growth beyond a certain point
Functionality
- When I click the menu links (Daily, Weekly, Monthly), nothing happens? I didn't have time to review your JS thoroughly, but there's apparently something not working there.
0 - P@Mustapha909Submitted 25 days agoWhat are you most proud of, and what would you do differently next time?
I'm most proud of figuring out how to build a more complex layout and discovering the techniques needed to bring my design to life. It was a challenging process, but every obstacle taught me something new about responsive design and CSS. Next time, I'd like to streamline my workflow even more and experiment with advanced layout methods from the start.
What challenges did you encounter, and how did you overcome them?One of the main challenges I encountered was structuring the layout correctly, especially ensuring responsiveness across different screen sizes. At first, some sections didn’t align as expected, and spacing issues appeared when resizing the window. To overcome this, I experimented with CSS Grid and Flexbox, adjusted breakpoints, and carefully inspected the design details. Debugging with browser dev tools and referring to documentation helped me refine the layout until it looked just right.
What specific areas of your project would you like help with?I’d appreciate some help fine-tuning my media query breakpoints. I’m looking for advice on setting the optimal screen sizes and ensuring a smooth, responsive layout transition between desktop, tablet, and mobile views. Specifically, I'm interested in learning how to adjust the breakpoints to better match the design requirements and handle edge cases where elements might not behave as expected.
P@jonatan-samuelssonPosted 21 days agoWell done, looks nice.
Like you say, there are some layout issues. First of all, the success message is too large at desktop sceen size, and the spacings are off. Secopnd, at mobile size, the success message gets very stretched out along the vertical axis. Try constraining the containers vertical size and provide consistent spaces between elements to ensure the layout works as intended.
Overall, the layout looks fine at small and big screens, but it breaks completely at medium sizes (widths between 480px and 700px).
One small thing is that the submit button doesn't display its active state when the user fills out the form. If you want to achieve this, use the ~ selector on the input:focus pseudo-element.
The JS is really good, particularly the validation process. One small tweak is to insert the actual email from the form into the success message in stead of the ash@loremcompany one. Easiest way I think to do it is with an empty span which you then fill using JS befor dispalying the message.
Marked as helpful0 - @guardianprimeSubmitted 2 months agoWhat are you most proud of, and what would you do differently next time?
I added Javascript to a frontend project!!!. hopefully it's going to be the first of many. I think my implementation might not be very efficient especially the Javascript. I will review the code when i get more knowledge of Javascript.
What challenges did you encounter, and how did you overcome them?responsiveness for different screens was so challenging especially devices like the samsung s8. media queries and good use of clamp() in my css helped.
What specific areas of your project would you like help with?i'd to see if there is a better approach using javascript to create the popup for both mobile and larger screens.
P@jonatan-samuelssonPosted 22 days agoHi!
Well done, good for you getting the JS to work.
A few comments and suggestions:
Layout, design, CSS
- Comparing your site to the designs, your element is different in size and spacing. Try to get fonts, padding and margins to match the design by pulling it up side by side to your page to compare them.
- One major problem with the above is that your component extends the whol viewport width at some sizes, which means part of your share-toast will overflow the screen when activated.
- Don't use your main as your component. Rather make a class like "article-preview-card" or something, which then goes inside the main. With your current setup, your component is always 70vh high wchich looks really strange, especially on mobile sizes.
- Your media query breakpoints are a bit awkward. Going from mobile to tablet for instance causes a kind of wierd layout. A dugesstion for positioning your content is to have your main wrapper be 100% wide and give it a min-height of 100vh, then set it to flex, justify-content: center and align-items:center. That way your component will always be in the middle of the page. Also: you have not made separate tablet and desktop layouts, which was in the design spec.
- Your use of clamp() isn't working in some places. For instance, .profile-div has
height: clamp(15%, 22%, 30%);
, which will always become 22%. Clamp will use the middle value (oft referred to as 'preferred' value) as long as it doesn't compute either to smaller than the first (min) value or larger than the last (max) value. If all values are the same unit clamp won't work at all. - The share-toast looks good, almost like the design. You should have another look at font color and text-transform though, as the "share" text in your page is white and not uppercase.
Javascript
You've made it work, which is really good. Well done. There are simpler ways though, as you don't need all the screen size checks in here, and you can basically just toggle the share-toast visibility and the share button active classes on and off, and then restyle the share-toast for different screen sizes using css instead. If you want an example, here's how I solved it: GitHub repo | Live Site
Marked as helpful0 - P@Oliko136Submitted 23 days agoP@jonatan-samuelssonPosted 23 days ago
Super nice, well done.
One thing I noticed was that your font sizes do not change with the sceen size. In the figma file some of the text presents change with screen size. Most notably the H1 in the hero section changes quite dramatically, whereas yours stay the same.
0 - @muhammadawaislaalSubmitted 25 days agoWhat are you most proud of, and what would you do differently next time?
i can do any challenge because programing is my passion
P@jonatan-samuelssonPosted 25 days agoHi!
OK first attempt, but I can't help but notice that most orf the components in your solution are different from the design. The challenge is genereally to make it look as close to the design as possible. This includes, for instance, using the same font-families, font-sizes, colors, paddings, margins et cetera. You have not done this for whatever reason. To get the most out of frontend mentor, I suggest you read the design guides carefully and always reference the design images when styling your project. Happy coding!
Marked as helpful0 - @id-nyntSubmitted 26 days agoWhat are you most proud of, and what would you do differently next time?
I get to know more use of grid and layout. Hope that I can apply them faster next time.
What challenges did you encounter, and how did you overcome them?There are 2 biggest challenges in this project:
- Layout using grid: In the beginning, I intended to divide the layout in columns then in rows and then in columns. But then I found out the use of column-span in grid, which made it more efficient.
- The quotation decoration: I spent quite much time to make it behind the text
I still don't really understand the position in relative and absolute meaning. And is there a better solution to make a background with icon on it? I inserted the icon in HTML and styled it in CSS. Any suggestions are welcomed. Thanks a lot!
P@jonatan-samuelssonPosted 25 days agoNice work. Some suggestions/comments:
##Html structure
- Your elements are in the wrong order. This makes them appear in the wrong order in the layouts
##Grid layout
- The tablet-sized design does not match the design spec. All your cards are the same size, when two of them are supposed to be one-column only
- Responsive and fluid sizes are generelly a good idea, but at some screen size transistions your site looks awkward. (example: 780-800 pixels wide) -Grid-tips: To make your layout work better, start by ordering the elements in the intended order. Then look into using grid-areas to position your cards where they need to be for the different sizes.
##The decorator image You specifically requested help on this, so here goes:
-
The decorative image moves around alot when the container changes in size. Notice also that it is visible even at mobile size, which is not the case in the design spec.
-
You can solve this by making it a background image. The following properties are useful:
-
background-image: url(images/image.jpg)
-
background-size: [two values for x and y size resepctively]
-
background-position: left [insert value] top 0
-
background-repeat: no-repeat
Marked as helpful0 - P@MarionCtsSubmitted 26 days agoWhat are you most proud of, and what would you do differently next time?
For this challenge, I wanted to practice a responsive approach by using the
What challenges did you encounter, and how did you overcome them?clamp
property for text andmargin-block/margin-inline
instead of the traditionalmargin-bottom/margin-left
properties. This is particularly useful when a page needs to be translated into a right-to-left (RTL) language, for example.I had a bit of trouble understanding how the
What specific areas of your project would you like help with?clamp
property works, but I managed to figure it out and I got the result to be as close as possible to the original design.I would be grateful for any kind of feedback!
P@jonatan-samuelssonPosted 26 days agoClosest to perfect I've seen on here so far :)
0 - @karlalasluisaSubmitted 26 days agoP@jonatan-samuelssonPosted 26 days ago
Nice work so far, looks very similar.
Except: it's much smaller that the design. I suggest going back over font sizes, paddings, margins, line-heights and container widths to achieve a vloser match.
0 - @vanya-vbSubmitted 26 days agoWhat specific areas of your project would you like help with?
I need help with the alignment of the text in the Instructions section. I couldn't make the text aligned left and it goes under the numbers on the next row, which is not right according to the deign. Please, advise.
P@jonatan-samuelssonPosted 26 days agoOverall looks good. One suggestion though is to pay closer attention to font sizes and spaces (margin/padding) to make it look even more like the design.
For your question re the instructions section, the css property you're looking for is
list-style-position: outside
- add that to your ol or corresponding class and you should get what you want.Marked as helpful0 - @umenorinSubmitted 27 days agoP@jonatan-samuelssonPosted 26 days ago
Hi!
Good job, you've made it look mostly similar to the design. A few suggestions for improvments:
- Design
- Overall size: I had trouble here as well, mine got too tall. Yours is the opposit, way too short.
- Margins and paddings: This is the main reason why your size is off. Your spaces inside the recipie card are much smaller than on the design. Try tweaking them until it looks as close to the same as possible. Whitespaces really do make designs much cleaner and easy to read. Same goes outside of the card, where the top margin is much smaller than on the design.
- Font size: Hard to tell exactly, but I suspect your fonts are just a tad too small as well.
- The nutrition table: Apart from spacing and fonts, the table is off in two main ways. First of all, it doesn't stretch the full width of the recipie card (which is shown by the horizontal lines stopping short of the margin edges). Secondly, the horizontal borders are too thick. Try making them identical to the ones found in the rest of the card and it'll look better.
0