Ben• 770
@BenConfig
Submitted
Please let me know if there is any way I can improve my code, particularly the JavaScript. Thank you.
Robert Rynard
@Robert-Rynard
All comments
- Robert Rynard• 560
@Robert-Rynard
Posted
Looks great! Your time periods are staying clumped to the left side at larger sizes below the desktop view you may want to using something like justify content: space-around or maybe centering them. Nice job!
Marked as helpful
1 - Esteban Canaza• 200
@stebanc
Submitted
I'm trying to write clean HTML, SASS and Typescript code, so any feedback will be appreciated.
Robert Rynard• 560@Robert-Rynard
Posted
You may want to look at the data as it is not switching when I select the other time periods. Looks great though!
Marked as helpful
0 - Eduardo Sanchez Casares• 200
@eduscasares
Submitted
Robert Rynard• 560@Robert-Rynard
Posted
Looks good! You might want to look at your media query break points and switching at a larger size because your desktop is getting very compact at the smaller sizes.
Marked as helpful
0 - Harish Radhakrishnan• 40
@HarishPRK
Submitted
Hi Community, kindly provide suggestions for my final build of this Project. Comments are appreciated😊
Robert Rynard• 560@Robert-Rynard
Posted
Looks like the value of your ETH really dropped :'( Overall looks pretty good just gets a little funky if you drop a little below 375px screen size.
Marked as helpful
1 - GamuchiraiS• 130
@GamuchiraiS
Submitted
Please leave any useful comments. Thank you so much!
Robert Rynard• 560@Robert-Rynard
Posted
Might want to add some more margin or padding between you body and button and maybe consider putting a max width on the mobile version at larger sizes before the media query it can get quite stretched. Overall looks great nice job!
Marked as helpful
0 - PrajwalDhule• 100
@PrajwalDhule
Submitted
Robert Rynard• 560@Robert-Rynard
Posted
Looks great! The message still says last week when you switch to the other time periods. Nice job!
Marked as helpful
0 - Lorenzo• 150
@ingegnerlorenzo
Submitted
Any feedback? I'm not sure about the responsivness, I mean it seems to work but it's like meh...
Robert Rynard• 560@Robert-Rynard
Posted
I think the responsiveness looks great! Nice job!
0 - jjjayed• 270
@Jane72-boop
Submitted
Any feedback is welcome, thanks! I just have one question, how do you make it so that the facebook icon's border stay as a circle? I eventually transforms into an oval shape in lower screen sizes
Robert Rynard• 560@Robert-Rynard
Posted
I had the same issue with these Icons and would love to know the best way to do it as well. Font awesome has something called stacked icons which you can put a circle around it that way but would love to know of the CSS way of doing it better. https://markheath.net/post/font-awesome-circle-background this is the article I found while looking into it.
Marked as helpful
0 - Jayesh Deorukhkar• 130
@Jayesh2812
Submitted
Robert Rynard• 560@Robert-Rynard
Posted
Looks great! It's a little small and you might want to look at changing the message for the previous time period as they all say last week when you change between the time periods! Nice job!
Marked as helpful
0 - Covaci Alex• 120
@covacialex
Submitted
Robert Rynard• 560@Robert-Rynard
Posted
Did your stylings not get added?
Marked as helpful
1 - christopher II• 1,220
@donchriscorleone
Submitted
I know it is not 100%. Any feedback would help! :)
Robert Rynard• 560@Robert-Rynard
Posted
If you want to make the image more responsive you can look into the object-fit property and using cover. Maybe take a look at the shadow of the button as it makes it look misaligned from the input. You could also put a border around a wrapper of the button and input to help make them look more aligned. Great job!
0 - Matthew• 50
@Washemj
Submitted
Hello,
I struggled to complete the icons - making them go behind the background I couldnt do, and behind the card in front of them I did but struggled to get everything to line up. Any help on how this is actually done would be much appreciated!
Also how you get the text of the name to go next to the picture - I was trying by changing the display CSS but couldnt work out how to get two lines next to the picture.
Thanks
Robert Rynard• 560@Robert-Rynard
Posted
You can set your card to overflow: hidden if you want to hide the overflow of the image. Alternatively you may want to set that to a background image rather than an image in the html as it is for styling purposes and then it would not overflow as well. You could wrap the p and h2 in a div and then put display grid or flex on the parent so the children would be the image and the new div. Looks good!
Marked as helpful
2 - Daniel Chang• 80
@danielchang2001
Submitted
How can I improve my shadows? Also should I start using bootstrap more or stick with base css?
Robert Rynard• 560@Robert-Rynard
Posted
Depends on what you are looking to get better at! If you want to get better at just bootstrap then practice that but if you practice CSS you will get a better understanding of how bootstrap and other CSS frameworks work.
Marked as helpful
1 - Gabriel Pinheiro• 260
@GabrielFMPinheiro
Submitted
Robert Rynard• 560@Robert-Rynard
Posted
Wow looks great! If you want to capitalize the first letter without javascript you could also just style it with css using text-transform: capitalize which I think in tailwind you would add the class uppercase.
Marked as helpful
1 - Tyra Carter• 20
@teecarter16
Submitted
I was struggling with the box-shadow, any tips are appreciated.
Robert Rynard• 560@Robert-Rynard
Posted
What did you feel you were struggling with the box-shadow? If you are trying to have more shadow on the bottom and less on the side you can use a negative spread-radius to bring in all sides and then move it further down the y axis.
0 - Ming Li• 130
@Mingrkli
Submitted
I do have questions on how to remove the space between the column as the screen becomes larger as I'm not sure on how to remove the space with the column gap being already as zero. Other than that any feedback would be helpful.
If you have any questions you can post a comment here or on the video of my stream at: https://www.youtube.com/watch?v=1D7xGfABgdo&ab_channel=CoderCow and come on in some time so we can all learn together.
Robert Rynard• 560@Robert-Rynard
Posted
The reason there is a gap between them is you have the columns set to be 1fr each so they all grow and the middle one is getting larger than the content and the content is centered so it is growing without it. You can set your middle column to max-content and it should remove the gap. Looks good great job!
Marked as helpful
0 - Momen Abdelwadoud• 50
@MomenAbdelwadoud
Submitted
Robert Rynard• 560@Robert-Rynard
Posted
Do you have the actual card hosted anywhere?
0 - Max Ruuen• 290
@ruuen
Submitted
How readable is my SCSS code and my BEM class name choices in general? Please let me know of any improvements that can be suggested.
Robert Rynard• 560@Robert-Rynard
Posted
Looks good! You can set border-radius on the .page__container and set overflow to hidden that way you don't have to set it individually for the top and bottom sections or in the media query. Nice job!
Marked as helpful
1 - Lorenzo• 150
@ingegnerlorenzo
Submitted
I'm not sure the mobile view settings are good to go. The card is looking good, but I think I could have placed it better in the body. Can someone give me a hint on how to do?
Robert Rynard• 560@Robert-Rynard
Posted
I'm not sure how you mean placing it better in the body but the centering with flexbox looks good and works! Also it looks good at all the screen sizes I looked at. Great job!
0 - Abdul-Shakur• 10
@AbdulShakurr
Submitted
Robert Rynard• 560@Robert-Rynard
Posted
I would suggest simplifying your grid more you can do a grid with 4 rows and 3 columns with 1fr each and have your cards span 2 and move the first and last one to the 2nd line.
0 - PGraeme Byrne• 390
@grmbyrn
Submitted
First challenge using javascript went pretty well, but getting the site to be responsive was quite tricky so let me know if it looks okay or could be improved!
Robert Rynard• 560@Robert-Rynard
Posted
I would recommend letting it stay in the mobile view for greater widths. There seems to be a typo on you grid-template-columns where you are putting the numbers in () which is causing that property not to work. also you might want to look into the object-fit property to make your images more responsive. Hope this helps Good luck!
Marked as helpful
0 - Martin de Boer• 50
@martindb1988
Submitted
Hello community,
I would like some feedback for the following problems i'm dealing with.
- SVG background is not showing
- placement of elements within the .plan class.
- Improvements regarding the code.
I know this is far from done, but i really could use some community feedback.
Robert Rynard• 560@Robert-Rynard
Posted
You're background is not showing up because you are not using a relative link in your url. It should be ../images/ I would also recommend putting this on your body so it covers the whole page and you will need to apply some more background properties to make it look correct. To position the items in the .plan class you may want to consider turning it to display flex and positioning them that way You do not need to put .container before all of your classes as they should work without that.
Hope this helps and good luck with the rest!
Marked as helpful
1 - Aleksandra• 360
@alexattt
Submitted
Robert Rynard• 560@Robert-Rynard
Posted
Looks great! The top part of your site seems to be getting cut off right after the 900px media query that you might want to take a look at. Nice job!
Marked as helpful
0 @nogyuuu
Submitted
Hi, i'm always worried if my code is well structured and is easily comprehensible to others.. what y'all think?
Robert Rynard• 560@Robert-Rynard
Posted
Yeah I would say it's pretty easy to read. I got a little confused reading your CSS file though because of the indents on some of the classes. It made me think they were nested selectors like in SASS but seems like you were using those indents to indicate the BEM architecture. Not sure what the best practice is on that. But anyways solution looks great nice job!
Marked as helpful
0
