Abed Fetrat• 450
@abedfetrat
Submitted
Hey all! Added animation to the arrow and hamburger as an extra. Feedback is welcome
Sarah
@sarahc-dev
All comments
- Sarah• 835
@sarahc-dev
Posted
This looks great!
I love the image zoom animation at the bottom! Really nice touch.
1 - Niti Mittal• 260
@NitiMittal
Submitted
I am having issue while making my page responsive for exercise card. It will be good if someone can have alook over my code and let me know where I am doing wrong.
Sarah• 835@sarahc-dev
Posted
Hey!
I think the problem is you are using % for the width of your cards - so they grow and shrink with the screen and the flex-wrap can never kick in. If you change you cards to a width or max-width in rem, the flex wrap will then work as it should.
Also - if you want to use flexbox and not grid - I would create a separate container for the time tracking cards - and apply flex only to these (and not the initial profile card). Then you don't have the issue of trying to move the second row across.
Hope that helps :)
2 - rubbersouls• 180
@rubbersouls
Submitted
This is my first exercise done with HTML and CSS from frontendmentor.io The code is not perfect but it more or less corresponds to the model :D And it is responsive If you have any advice to improve my code I'm a taker, thank you
Sarah• 835@sarahc-dev
Posted
Hey! This looks good - so well done! A few tips that might help you on future challenges -
- Use min-height: 100vh instead of 100% on the body and don't set a height on the html tag.
- You don't need so many divs! If it only contains one element - you probably don't need it.
- Avoid setting a height and width on everything - try to use padding and margin to create the spacing instead.
- For accessibility, avoid using px values for font sizes, heights and widths - use rem or em instead
Hope that helps. Keep going!
Marked as helpful
1 - Tejaswini Labade• 150
@TejaswiniLabade
Submitted
Hi community members, one more amazing challenge I just completed. I would really appreciate your awesome feedback.
Sarah• 835@sarahc-dev
Posted
This looks great - congrats!
Just one tip - you should write your code mobile first and use a media query with min-width to make the amendments for larger screen sizes. It's best to get used to this way from the beginning :)
Hope that helps!
Marked as helpful
1 - Archie• 30
@archie-k
Submitted
Hey guys, please let me know where i should improve my css spaghetti code, any criticism and tips are welcome. Thanks you so much
Sarah• 835@sarahc-dev
Posted
Hey! This looks good - I wouldn't say you have spaghetti code at all!
- Using h1 instead of h2 in this instance would stop the accessibility report shouting at you - you wouldn't typically have more than one h1 - but in this design where the 3 main headers are of equal importance it doesn't cause a html error I don't believe.
- Use rem for your font sizes instead of px for better accessibility (allows a user to set browser to a larger font). But if you do this, you also largely need to set heights and widths if using in rem or em so everything's container can also grow if font is set to large.
- I wouldn't use justify-content: space-around to create the spacing between the text - i have a thin screen and everything gets squashed for me. Better to use padding or margins to ensure the space is always there on different screen sizes.
Hope that helps :)
0 A question that came while I was starting to build this, is:
-
If all sections have the same importance, how do you determine what heading to use since having multiple H1 headings is frowned upon, but in context, it doesn't make sense to have more than one level of heading or have a different type of tag that's styled the same as a heading?
-
I felt like using margin at the bottom of the paragraph to create the spacing was a bit of hack. Is there a better way to do that?
-
I wanted the buttons to remain in the same place regardless of the space taken up by the paragraph. Is there a way to achieve that without using absolute positioning?
-
Any tips to improve the legibility of my code would definitely be helpful.
Sarah• 835@sarahc-dev
Posted
Hey!
- My understanding is that if you are using semantic HTML like <article> or <section> and the heading for each is of equal importance such as in this design and there is no other main heading - it is ok to use multiple h1s.
- Using margin under the paragraph is what is giving you the height of the container. An alternative would be to set a height in rem or vh and use margin-top:auto on the button to push it to the bottom. You have to be careful of the text overflowing at different screen sizes if you do this - but it does keep the button in the same place.
Hope that helps!
Marked as helpful
1-
- Bouzar abderrahmane• 250
@AbdouHichem
Submitted
any suggestion is more than welcomed :)
Sarah• 835@sarahc-dev
Posted
This looks great! The only thing you could have done was added a media query to increase the spacing at the top on large screen sizes :)
1 - Maianne Thornton• 20
@MaianneThornton
Submitted
This is my second challenge, and it went much smoother than my first!
Any suggestions on how I can improve are very welcome! Thanks 😊
Sarah• 835@sarahc-dev
Posted
Hey! This is looking good. Just a couple of suggestions to help you - you don't need to use a media query for the mobile size. That should be the default, and only use a media query for larger screens. You have a weird situation where you lose the background and the card grows too big between 375px and the large screen media query. You can use a max-width on the card to prevent it growing too much before the media query kicks in.
Also add some blur to the box shadow to make it less harsh e.g box-shadow: 0px 20px 8px hsl(228deg 45% 96%);.
Hope that helps :)
Marked as helpful
0 - Igor Ostojic• 300
@igor-ostojic
Submitted
I'm proud of the work I've done on this mini app, but I have a question.
Was there an easier way of changing the data when a button is clicked ? I feel i may have over complicated things.
Thanks in advance !
Sarah• 835@sarahc-dev
Posted
Hey!
I also completed this challenge today. I have used one forEach() to loop through the JSON file and then used an if statement to update with the relevant data - rather than using a for loop on every element. Have a look at my solution, you may think it is an easier way (ignoring the additional complication of trying to animate it!)?
I like the way you have rendered the whole component using Javascript to make it more dynamic - I didn't think of doing that!
1 - Suraj Kandlakunta• 410
@Suraj1333
Submitted
Had bit of trouble with the responsiveness of the mobile design. Any suggestions would be helpful.
Sarah• 835@sarahc-dev
Posted
Hi
I would suggest not using a set vh or vw on your main and your text. I have a small macbook air screen and it cuts off most of your text because you've used a fixed height and it doesn't all fit on my narrow screen size. Use padding on the body instead to add the border area around the main, and allow the main content to grow as needed to suit each screen. You can use min-height, but not setting a fixed height on things should really help you with responsiveness.
Hope that helps :)
2 - Alain Moratalla• 135
@almoratalla
Submitted
Hey there! I am open to any suggestions or feedbacks. Please try this game as well if you like. I've also added a semi hard mode where it will try to guess your choices based from your previous choice pattern. Feel free to share any tips or suggestions. Any form of remarks will be highly appreciated. Thank you!
Sarah• 835@sarahc-dev
Posted
Wow, this is great! Love it :)
1 - Andrew Njoo• 40
@adnjoo
Submitted
how to have rounded edges for a responsive div?
Sarah• 835@sarahc-dev
Posted
Use border-radius :)
0 @harish250
Submitted
Please review my design suggestions are welcomed
Sarah• 835@sarahc-dev
Posted
This looks great!
My only feedback would be to design for mobile first - so use min-width in your media query, instead of designing for max-width. Also set the px value of your media query to suit your design - for example if I manually adjust the viewport, it overflows the screen before it adjusts to the smaller screen view.
Hope that helps :)
1- Syed Ali Mansoor• 460
@alimansoor-create
Submitted
First time trying out custom validation with Javascript and Sass. Please check it out and leave some feedback.
Also, does Github Pages not allow forms? Because I get a 405 Not Allowed page when I submit the form. Is there any way to just be taken back to the form instead of it being submitted somewhere?
Sarah• 835@sarahc-dev
Posted
Hi! When I use your form it just refreshes the page - so not sure if you found a way to fix this. You can also use preventDefault() in the JavaScript where the form submits to prevent the default functionality and not submit the form.
Hope that helps :)
1 @Zepticona
Submitted
Hello This is my first ever submission on the site. I'm not a newbie to coding web pages but I'm not confident enough as to whether I'm following the best practices or not. So I hope to get a review of what are the mistakes I made, what could be the best approaches and where from I could learn those things.
Sarah• 835@sarahc-dev
Posted
Looks great to me! :)
1@AnthonyRodriguez93
Submitted
I am having trouble putting the svgs onto the background but they work perfectly for the img tag I'm guessing they can't work with urls but mdn says otherwise.
I still have trouble with layouts and flex positioning and I get confused pretty fast once the divs and classes start piling up is that normal?
Thank you for any advice!
Sarah• 835@sarahc-dev
Posted
Hey! It took me a while to work out the problem with your background images - but it is because your styles.css file is inside a styles folder. So the css is looking for the image at the file path "styles/images/..." instead of at the root. Therefore you need to use "../images/..." so you are looking for the images folder in the parent directory. That should solve it!
I don't know what best advice I can give you about flexbox - I think you have to just think about the parent and it's direct children only and block everything else out. If you need to in something like chrome dev tools you can hide and show all of the HTML contents of say a card. Sometimes I minimise everything else, so I'm just looking at the parent and the direct children - and I find the highlighting when you hover over each one helps me to visualise what I want to do.
Hope that helps :)
1- Daniel• 310
@daniel-g-p
Submitted
Hi all, would anyone know how I can remove the huge amount of white space that appears at the bottom of the page on mobile view. Thanks:)
Sarah• 835@sarahc-dev
Posted
Hey! When I open the preview of your site it doesn't allow scroll. All of the text is pushed below what is viewable when I reduce the window size. The overflow: hidden is forcing the window to be zoomed in but disabling the ability to scroll.
I think the problem relates to the image sizing, you have set the image width to 200% but are trying to force it into a smaller space to achieve the crop (I assume). You would be better off setting the image as a background-image and positioning it with background-position.
Hope that helps :)
1 - Roger Lemos• 20
@RogUnique
Submitted
Hi guys,this is my first challenge and really was a challenge.
I have some questions: How I can decrease the media queries use ( it's necessary?)
In mobile devices I noticed that when the buttons (switch and slider handler)are clicked they are select, someone know if is possible remove that?
Now about cursor , I don't understand how add the blur effect.
That's All if someone have anymore feedbacks I'll be happy. Thank you very much in advance for the platform and possibles contributions.
Sarah• 835@sarahc-dev
Posted
Hey!
This looks like a great effort! Really close to the original design. Good work on completing it.
To reduce the number of media queries, you can design the CSS for mobile first, and then use max-width. So the container grows up to the max-width and then stops growing. I looked at your code, but I actually couldn't figure out why so many media queries were necessary. I think it could be related to how you are setting the width on the pricing component or it's related to setting a height. If I recall correctly, I learnt to avoid setting heights as it can cause problems. I tend to set a min-height: 100vh; on the body but I don't set height on anything else - unless it needs to be a fixed height in px - like the toggle.
If you want, have a look at my code for this challenge - I only used 1 media query in my solution.
You can remove the default focus states by using the psuedo selector :focus in CSS (so button:focus) and setting the outline to none. In the real world, for accessibility there should be some indicator that the item is selected when tabbing through the website however.
To add the blur effect, you can use box-shadow and increase the 'blur' (the third number) when defining the box-shadow - eg. box-shadow: 0px 15px 30px rgba(0, 255, 231, 0.6); The 30px defines the 'blur'.
Hope that helps :)
2 - MasterKrab• 940
@MasterKrab
Submitted
I am just a beginner and would appreciate any feedback.
Do I have good practices? What errors do I have? How could I improve my code?
Thanks <3
Sarah• 835@sarahc-dev
Posted
Hey!
I really like how you have implemented the toggle and the transition in the theme switcher.
Just as a personal preference visually, I don't like how the containers stick so tightly to the sides - especially when resizing the window. I preferred to keep a set gap, and have the margin grow before a new container could fit. But it's all personal preference I guess! Other than that, it looks great!
1 - Ala Abd Elrahman• 110
@Ala161092
Submitted
Hi Everyone,
It has taken me hours upon hours of frustration (mainly me bashing my head against the screen) butttt i think i have finally completed my first project! I would really appreciate if anyone can spare me a few minutes by looking through my code and giving me any tips/advice on how I can better improve my html and css?
I REALLY struggled with the background images and getting them into the right position, i'm not sure if my method was correct or if there was a better way of doing it, which i'm pretty sure there must be?
Anyway any tips/advice would be much appreciated - many thanks in advance!
Sarah• 835@sarahc-dev
Posted
Hey!
Good effort! Congrats on finishing it, and getting it uploaded!
I think it is better to use background-image in CSS for the background circles, rather than adding the images to the HTML, as it is purely decorative. You can size and position the images in CSS (in a similar way to what you have done), using background-size and background-position and it also allows you to add a media query (or queries) to change the size and positioning for smaller or larger screen sizes.
Hope that helps :)
1 - Emmanuel Motsi• 10
@motsione
Submitted
How can I get the background to shrink with the background images so that they maintain the same design as on desktop. My background images were disappearing as I was reducing screen size.
Sarah• 835@sarahc-dev
Posted
Hey!
You will need to use a media query to amend the background size and position on different screen sizes. Ideally, you should design for mobile first, and then add a media query/queries for larger screen sizes.
Hope that helps :)
0 - Sebastian Söderström• 160
@bastiman85
Submitted
All feedback is welcome!
Sarah• 835@sarahc-dev
Posted
Looks great! I couldn't figure out how to get the overlay on the creations, but now I see how you did it. Thanks! :)
0 - Sylvain• 70
@longani974
Submitted
Hi, it was great fun coding this challenge. But I don't know how to deal with the effect or mask of the image background for the mobile version. I tried different solutions and kept the one that I prefer. If anyone has a suggestion on this particular point, I would appreciate it for sure. Thanks for reading me and looking at my solution.
Sarah• 835@sarahc-dev
Posted
Hey!
When I view your site, it doesn't scroll down to reveal the form part. You should set min-height, instead of height to 100vh. Then on different screen sizes, it will allow you to scroll if it doesn't fit in the view. I believe you don't need to use 100vw as well.
For the mask effect, I used: background: linear-gradient(rgba(18, 23, 37, 0.9), rgba(18, 23, 37, 0.9)), url(./assets/mobile/image-host.jpg);
Then on the larger screen sizes I used a media query to change the background to the solid color.
Hope that helps :)
0
