Fran Extremera
@franexmo81All comments
- @girldocodeSubmitted about 1 year ago@franexmo81Posted about 1 year ago
Great job with this challenge. Nice and clean.
I've only noticed a couple of things that could be improved:
-
User info (name and Verified Graduate) are aligned on left on the design, but appear centred on your solution.
-
You could have grouped some properties for the profile images, like this:
.profile-image{ height: 28px; width: 28px; border-width: 1px; border-style: solid; border-radius: 50%; margin-right: 13px; } .dan { border-color: var(--White); } .jon { border-color: var(--Moderate-violet); }
Also, remember to replace Your name here on the footer 😉
Happy coding!
1 -
- @devaramnyeSubmitted about 1 year ago
I used this project to reinforce my Grid knowledge as I started with grid a few days ago. I am really happy with the result.
Any feedbacks are welcome! have a beautiful day!
@franexmo81Posted about 1 year agoReally nice job.
The solution looks as the intended design and the responsiveness works perfectly.
If you want to, you could simplify your CSS a bit by grouping similar elements with the same class name. For example:
Here is some code from your solution:
<div class="one" id="one"> <div class="two" id="two"> .one { background-color: var(--moderate-violet); background-image: url(./images/bg-pattern-quotation.svg); background-repeat: no-repeat; background-position: 90% top; border-radius: 10px; overflow: hidden; padding: 1.5rem; } .two { background-color: var(--very-dark-grayish-blue); border-radius: 10px; overflow: hidden; padding: 1.5rem;
What you could do is assign an additional common class to both divs that contain the common properties, like this:
<div class="card one" id="one"> <div class="card two" id="two"> .card{ border-radius: 10px; overflow: hidden; padding: 1.5rem; .one { background-color: var(--moderate-violet); background-image: url(./images/bg-pattern-quotation.svg); background-repeat: no-repeat; background-position: 90% top; } .two { background-color: var(--very-dark-grayish-blue);
This could be applied to the rest of the elements (userone/usertwo, infoone__p__s/infotwo__p__s ...)
This brings some advantages. Let's say that in the future you want to modify the padding for all the cards. You would only need to modify it in one place.
I hope you find this useful and have a successful learning.
1 - @JessicaSamtaniSubmitted about 1 year ago
Hello all, learnt Grid from the best teacher ever. Had lots of fun and grasped the concept. Still lots to learn, excited. Open to feedback. Thanks
@franexmo81Posted about 1 year agoNice submission, really accurate.
I've just noticed some misbehaviour in the responsiveness:
- The cards are not vertically centred on the screen.
- The text overflow the cards on screens between 795 and 1250 px width
- The violet card doesn't take the full width on screens narrower than 795 px
I think that most of the above can be fixed by just doing:
- Remove
padding: 70px
for main-wrapper - Replace
height: 100vh
withmin-height: 100vh
for main-wrapper - Remove
grid-template-rows: 300px 300px
for testimonial - Remove
width: 300px
for the violet card under 795 px width - Remove
margin-top: 500px
andmargin-bottom: 500px
for main-wrapper under 795 px width
I hope the above helps you in your way to become a successful developer. Good luck!
0 - @Dave-n-techSubmitted about 1 year ago@franexmo81Posted about 1 year ago
Nice job. The grid layout works as intended and is responsive.
However, I'm afraid that the font type is not properly linked. In Index.html you have:
<link href="https://fonts.google.com/specimen/Barlow+Semi+Condensed" rel="stylesheet">
But, actually it should be:
<link href="https://fonts.googleapis.com/css2?family=Barlow+Semi+Condensed:wght@500;600&family=Roboto:wght@400;700&display=swap" rel="stylesheet">
Please, have a look to how to use Google Fonts to understand why the above is needed.
Also, at the CSS, it's recommended to place the font type between quotes, and provide a more generic second option. I mean, instead of what you have:
font-family: Barlow Semi Condensed;
To have this:
font-family: "Barlow Semi Condensed", sans-serif;
I hope these changes helps. Happy coding!
0 - @mohamadshawabkehSubmitted about 1 year ago@franexmo81Posted about 1 year ago
Good job overall.
Just some things that don't look completely OK:
- The background colour doesn't match with the design
- The cards are not vertically centred on the screen
- There is no gap between cards when width is less than 600 px.
Also, I would personally improve the footer although it's not part of the challenge.
All above are small details that I'm sure you can easily fix and would lead you to a perfect page. Well done!
0 - @dnksebastianSubmitted about 1 year ago
My second project created in React. The game score is saved in the localstorage, I also added a button to reset the game state and a toggle switch to change the game mode.
@franexmo81Posted about 1 year agoCongratulations for such a great job. It looks amazing and works perfectly.
I've only notice some few details that maybe could be improved:
- The shadow when hovering on the 3 (or 5) options to choose would look better with some transition
- You could take advantage on the link for your name ("Sebastian") at the footer by making it point to your personal webpage, or Github profile...
Other than that, excellent job. Keep it up!
Marked as helpful1 - @RoRajakSubmitted about 1 year ago
Hello Guys,
Hope you are doing well. I made lots of mistake specially in responsive design so if you able to suggest or improve my coding. It will be helpful
Thank you in advance
@franexmo81Posted about 1 year agoHi RoRajak and congratulations on submitting this solution.
Don't be so hard with yourself ("I made lots of mistake") 😊.
The layout looks quite well and accurate on desktop view. However, it looks like you struggled a bit with the mobile one. I'm sure that if you keep trying, you'll be able to improve. It's all part of the process.
Here are some other easy fixes that would help:
I've noticed that the intended font is not working.
Instead of linking from the TTF file:
@import url(/results-summary-component-main/assets/fonts/HankenGrotesk-VariableFont_wght.ttf)
Try better with importing from Google Fonts:
@import url('https://fonts.googleapis.com/css2?family=Hanken+Grotesk:wght@500;700;800&display=swap')
Also, it looks like the images are not loading properly.
Try to replace the way you've linked:
src="/assets/images/icon-reaction.svg"
With this way:
src="assets/images/icon-reaction.svg"
I hope all this helps. Good luck and keep going!
Marked as helpful0 - @Dng120696Submitted over 1 year ago
any comment so I can improve my skill thank you
@franexmo81Posted over 1 year agoYour solution looks really similar to the design. The responsiveness seems to work nicely. Well done!
I would have named the CSS variables in such a way that their name is not referred to the color but for their use. Something like "primary-color" or "accent-color", etc.
The idea behind it is that if the colors need to change in the future, you can simply modify their value and they will keep their meaning without needing to modify their names all along the file.
Bonus tip for a better appearance: Some transition for the button hover status.
Marked as helpful0 - @NehalSahu8055Submitted over 1 year ago
👋Hello, Frontend Mentor coding community.
👨🏼💻This is my solution for the
Singel-Price-Grid-Component
.I'll be happy to hear any feedback and advice!🤗
@franexmo81Posted over 1 year agoHi!
It looks good, but here are a couple of easy wins that could easily improve it:
- Instead of
DIV
, make it aBUTTON
and give it aHOVER
effect. - Increase the
@media
querymax-width
to about700px
to improve responsiveness
Bonus one: Some shadow on the card as per the example.
💪
Marked as helpful1 - Instead of
- @Ecrb3Submitted over 1 year ago
Hello guys, Can you tell me how can I change the svg colors
@franexmo81Posted over 1 year agoHere is an example of SVG changing colours when hovering:
Basically, you need to include the SVG definition inside the html document, instead of linking them from it. Then you select it and modify
fill
with:hover
pseudo-classI hope it helps
0 - @itsale-oSubmitted over 1 year ago
Hi!
This is my solution to the profile card component challenge. Please, feel free to point anything I could've made diffrently.
@franexmo81Posted over 1 year agoHi Alessandra. Nice looking job.
However, I've noticed that the card doesn't stay in the centre for screens wider than 1440 px.
I believe the reason is
body
hasmax-width
so it doesn't fit the whole width of the screen and stay on left side.In any case. Good work, keep it up!
Marked as helpful0 - @vBenTecSubmitted over 3 years ago
Hey everyone, Thanks for reading over this. I invite you to have a look if you are interested in my solution to finish the challenge. I tried to make my page responsive for the common sizes. I am open to anyone who wanna share some review exchange. Let´s learn and help each other.
Greetings ben@franexmo81Posted over 3 years agoHi Ben, your solution looks really good.
I just noticed that the buttons lose their shadow colour when hover over them.
Nice job!
0