Ervin Natareno• 70
@ervinCodes
Submitted
I found myself using multiple for loops to access similar data. Is there any way I can refactor this using less code?
Rafal
@grizhlieCodes
All comments
- Rafal• 1,395
@grizhlieCodes
Posted
Hi Ervin,
I made a video covering how I did this challenge, a lot of people seem to have found it useful. I'm pretty sure I explain the JS as I'm going through it.
In case you want a sneak peak of the code, here is the link to my GitHub repo.
Generally there is nothing wrong with using a for loop though.
The alternative you can try is the forEach loop.
It works like a normal for loop in a way, it usually takes in an array of items (lets say cards or buttons) and you can just run any code you want.
You have access to the index of each item as well as the item itself so for example:
buttons.forEach((button, index) => { button.textContent = `Button num ${index + 1}`; })Whether or not it uses less code: probably. But I'd concern myself with readability personally rather than length. The less work your future self/someone else has to do to understand the code, the better. I've seen people obsess over quantity of code and they sometimes go as far as to sacrificing readability (I'm not saying you do this!), and man it's tough to work out what's what. Especially in the evening after a bunch of other work 🥲
Happy learning 👍!
P.S. I go into some CSS that you may find useful in the video for some additional tips.
1 - Rod• 260
@bague-rodnel
Submitted
I'm not good with color theory. For now my thought process. is "just switch to light/dark shade of the same color and try to match how 'far' the color is from black/white". lol.
Share some good resources about the topic. :)
Rafal• 1,395@grizhlieCodes
Posted
Super awesome for you to practice extra stuff with this project, that's the spirit I LOVE (i tend to maximise my learning per project also).
When it comes to color, I struggle also.
From what I can tell is that you can't simply 'reverse' colors and hope for the best. Sort of like the complete invert-standard that seems to be prevelant amongst newer designers/developers. You need to find a similar 'theme' color and build around it. It can also be said that for this project you might have kept this card identical, even though the background changed since it's a nice contrast.
Imagine you had 3 cards, this would be the center, and the cards on the left/right would be the ones with the white/light-gray background. All of a sudden you've made this stand out massively. I'm just saying it's not as simple as one may think.
Feel free to criticise my website's dark/light mode toggle and tell me what you think. I think we can discuss and learn from the criticism. I'll be honest, I spent 5% of my time implementing the light mode and coloring so it probably sucks. I just wanted it to seem bearable.
I also like discussing stuff :).
An interesting resource is material ui (I think this is basically a google built team that creates AMAZING resources and guidelines for us devs/designers). They cover their thinking/principles/fundementals in incredible and useful detail :)
Marked as helpful
1 - PSara Dunlop• 220
@Risclover
Submitted
As always, feedback is much appreciated!
This took a little longer for everything to come together, but near the end, I got on a roll where I was able to figure out nearly all of the more difficult aspects pretty quickly (the images in desktop mode, the arrows turning upside down when you click on a question, etc). I'm glad I got to practice some JavaScript!
As a side note, I'm using a TV as an external monitor because my laptop screen is broken. Unfortunately, that means that sometimes my projects can look smaller to you guys than it does to me. My proportions are correct on my end, but I'm afraid that everything translates to be a bit small for everyone else. I have to figure that out. -_-
Rafal• 1,395@grizhlieCodes
Posted
Overall a great solution, well done on everything! I agree with Rod on all points - hope you get yourself a new tool/fix your screen too :)
I think it would be more optimal to centre the images (Desktop view) in the vertical centre. If you open up all of the suggestions it leaves this odd white-space on the bottom of the images that I have to throw evil-eyes at 🤩.
If you want to practice even more javascript here, animate the anwsers to the questions when you click on a question. It's a fun exercise and I learned a ton from it that I honestly thought I already knew hehe.
To me transitions/animations are pretty important, almost as a part of accessibility now so I usually pay special attention to them. I also just find it 'nice'. If you need any advice/help with that just let me know. Or look at my solution for this to get a better idea. I was very.. let's call it specific with naming my JS variables just so I could later understand my code. It felt basically written in English by the end of it lol.
0 - Rafal• 1,395
@grizhlieCodes
Submitted
Hey all,
(Question on the bottom, summary on the top).
Just one I did for fun. I realised I could practice building keyboard-accessible navigation with some thought given to tabindex and when to add/remove it from the collapsible links. Then I realised how badly I wrote my JS and refactored it to be more readable.
I gave special attention to the transitions/animations for the header. You will see the collapsible menu-categories transition themselves from the direction/location of the buttons that activate them. I also added smooth transitions for the actual link-containers as well, the
max-height,gap,paddingandmarginproperties together is what makes the smoothness possible.Also learned about what exactly scrollHeight is and that it does not seem to include some aspects of the content, I believe it was omitting/excluding
line-heightof my<a>'s for example. It also ignores things like thegapproperty of any parent flex container. So I just wrote my owncustomScrollHeightvariable. All in all I didn't expect to learn so much, as minor as it may seem at first. AWESOME project.I paid attention to making sure the grids I used were smooth. I did NOT pay attention to the background image for the hero, just sort of threw it in there with rough calculations, I'm sorry :(.
Question: Does anyone have any good thinking-framework for working with CSS grid? Some sort of a mental model with which to follow and break grid-down? My current style is based on analysing the design and then just fitting the grid in, mobile first with an OK understanding of the various grid-units we can use for rows/columns. But beyond that it's laissez faire.
Cheeeeeeers 😁
Rafal• 1,395@grizhlieCodes
Posted
Oh yes - I'll probably correct minor styling things shortly. I seem to have omitted quite few 'details'.
0 - Benja.min• 740
@BenjaDotMin
Submitted
Hello all! Today I learned about managing global state, using React and Redux. As always, pointers are welcome!
Thanks very much.
Rafal• 1,395@grizhlieCodes
Posted
Awesome stuff man, love how close you got with the final result. Must have been satisfying when you saw that generated screenshot comparison 😁
I don't have much feedback in terms of code but here's an idea regarding accessibility:
- If you click on the cart you currently have to click on it again to close it.
- Implement a listener + function that closes the checkout when the user clicks the 'Escape' key.
- Also if the user clicks on anything that is not the cart, whilst the cart is open. Easy to do with another global listener (perhaps one that's created and destroyed depending on cart state) and
closestlink.
A few pet peeves
-
headeris located inmain, any reason for putting it in main instead of thebodyor rather the container that you work with in React? I use Sveltekit but looks like similar concepts apply, some div or whatnot that acts likebody. -
I personally avoid using fixed heights. Any reason to use it for the
header? I usually build any height I need based on the content + gaps + paddings etc. -
Did you build the carousel? Wondering why it doesn't just loop when you reach the last/first image and then continue clicking in the same direction. Eg.: If we're on the first image and click
previousit doesn't go to the last image. I'm only asking because I recently tried building it from scratch along with the transitions etc 😅
Awesome job though. Managing global state is when you start seeing the possibilities I think, I sure did.
Marked as helpful
2 - Duyen Nguyen• 950
@Duyen-codes
Submitted
I really need help with the unsolved: -header gradient background, when adding the second background with hsl, the background pattern disappeared, but with rgba(in this case i use kind of purple, it shows) -JavaScript for toggling navbar menu, burger icon and arrow icon, the menu showed very quickly and disappears right after that. It doesn't stay. -phones image stretches the whole container in art section and I don't know why.
Please tell me what I did wrong! I know there are other minor details that need to be fixed but for now I think about the above. Thanks a lot!
Rafal• 1,395@grizhlieCodes
Posted
Real nice solution so far apart a few minor things you already mentioned :)
I'll makes notes one by 1 and reply to this comment chain as I spot stuff 👍
Your
burger-iconshould be a button as it activates some JS and doesn't take the user anywhere in reality, instead it opens up a modal. Because it's currently a link it refreshes the page every time you click it, hence the menu showing up for 1 second. If you look at the browser icon on the top you'll see that the page is refreshing.Marked as helpful
0 - Duyen Nguyen• 950
@Duyen-codes
Submitted
One thing I haven't figured out is the footer on desktop mode becomes have its own space somehow. Been sitting on this problem for more than an hour. I'd be grateful if you can have a look at my codes and direct me to the right path. Thanks a lot!
Rafal• 1,395@grizhlieCodes
Posted
Nice solution!
I'm not sure I understand the issue exactly, the footer looks fine to me unless I'm missing something obvious 😅.
What do you mean by 'own space'?
Marked as helpful
0 - Duyen Nguyen• 950
@Duyen-codes
Submitted
I hardcoded the data since I got stuck at injecting data from JSON file. Would really appreciate if you can give some tips on this problem. Thanks a lot.
Rafal• 1,395@grizhlieCodes
Posted
Done. Link
I was editing on a new computer and new software so took longer than expected and is far from perfect imo - kept on hitting my head against sound issues 😒.
I think I covered the main gists you were looking for. I kept the JS very minimal and avoided some refactoring, for example fetching the data like Naser suggested. I did that initially then realised I'd be getting into the world of fetch and promises and thought otherwise to keep this beginner friendly.
I'd love any feedback/critique within YouTube if you end up watching it. You can find the JS bit within the video chapters.
0 - Duyen Nguyen• 950
@Duyen-codes
Submitted
I hardcoded the data since I got stuck at injecting data from JSON file. Would really appreciate if you can give some tips on this problem. Thanks a lot.
Rafal• 1,395@grizhlieCodes
Posted
Just built and uploaded my solution for this. Video will probably be made by tomorrow's end, will do most of it today evening. Sunday latest..?
Marked as helpful
0 - PrajwalDhule• 100
@PrajwalDhule
Submitted
Rafal• 1,395@grizhlieCodes
Posted
Hi Prajwal,
Great attempt! As the most solid piece of advice, straight from the get-go:
Avoid using height. If you want to use
heightI recommendmin-height. Why? GREAT QUESTION! The concept here is simple: we want to let the content decide how large a container is. When we write CSS we are working with alot of unknowns: What is the screen size of the user's client? What browser and what version are they using? Do they prefer darkmode or default? Etc.This gets into the core of FRONT END web development: we don't want to create pixel-perfect styling for the content. We want to create adaptable content that is responsive.
I actually made a video doing this project that will do a better job of sharing 'good approaches' (imo). Here's the link in case you're interested.
Feel free to bombard me with questions, I love this stuff :)
Cheers!
0 - Duyen Nguyen• 950
@Duyen-codes
Submitted
Rafal• 1,395@grizhlieCodes
Posted
Nice solution Duyen :), I like the use of grid!
A few shortcuts/tips:
When you want to declare a grid that will only have 1 row and 1 column AND will center its' child/ren all you need to do is add this code to the parent:
.grid-container { display: grid; place-items: center; }That will do it. So I'm referring to your
.banner-overlayfor that. Oh yeah, if you inspect your code you will see that yourgrid-template-columnsandgrid-template-rowsare actually not working. You can't declare just1but you can declare1fr😁, then they would work fine. I think I also saw this mistake on.banner.I'd also look into using
flexwithflex-flow: column nowrap;andgapto streamline stuff a little bit. I see that some of your elements have paddings/margins etc. I usually run a css-reset file that removes all default styling (especially margins/paddings) from ALL elements. Then I write them fresh.Overall awesome job, I assume you wanted to improve in
grida bit with this project? Love using it personally, it gives you such a controlled 'environment' to position elements with.Oh ya - I recorded me doing this solution, in case you want to check out what I might have done differently: link, only sharing because I think it might be beneficial, idc about 'views' as much (yet).
0 @Drago78624
Submitted
Here's my first FrontendMentor project completed by using Flexbox mainly Please let me know how can i improve, i always have a thought that im doing something unnecessary here and there😅
Rafal• 1,395@grizhlieCodes
Posted
Hello!
I know how you feel with 'doing something unnecessary'. That was at the forefront of my mind when I started coding and didn't know what I didn't know 😁
I started writing a bunch of stuff but I thought it'd be easier to just show you. So I made a short video, hope it's helpful.
I did also make a video recently of me doing the entire challenge (with SCSS, not sure if you're familiar with it) if you're interested: link.
1- Aaron Currie• 100
@AaronCurrie
Submitted
I was using this as a chance to practice css functions like clamp to make the site as responsive as possible. I was also using variables for the colors to get used to using these more. I am still learning layout and how to create the best layout, this design was quite simple and didnt need much. There were a few instances I used flex box to create an inline row but these were relatively simple. What other ways would you do the layout?
I did struggle with the main image overlay but managed to come up with a solution that worked eventually. What would be the best way to go about doing the image overlay and does the rest of my code make sense?
Rafal• 1,395@grizhlieCodes
Posted
Hi Aaron,
I started writing stuff and i realised it would have been quite long so I just ended up recording a video lol. Here's the link. I just try to share some concepts basically that are best practice and overall time-savers, win/win.
I actually created a video on this particular challenge a few days ago, but I did use SCSS(sass) so it may look slightly alien from normal css. (It's not tbh and worth learning asap if you ask me, SCSS is something you can learn whilst learning CSS without hindering your effort). Link in case you may find it useful.
Ah - yes, in terms of the overlay, completely forgot - sorry. That's actually covered in my 'tutorial' (second link). If you're uncertain of the exact code (because SCSS may look weird) then just go to my github repo for the project and it should make sense if you open the
.cssfile. Shout if you have any questions.Cheers :)
Marked as helpful
0 - Duyen Nguyen• 950
@Duyen-codes
Submitted
One thing remained unsolved. When I scale down the viewport from desktop view, there's a line on the right side of the grid container. I couldn't figure out why and how to fix that. I'd be thankful if you can look at my codes and give me a hint on where and how to get rid of that line.
Rafal• 1,395@grizhlieCodes
Posted
Oh yes - if you do
display: noneon theul.benefits-listyou will see that the problem (line) also disappears. Or if you removepaddingfromsection.benefits. But these of course don't solve the real issue so I thought the above solution was apt to mention.Marked as helpful
0 - Duyen Nguyen• 950
@Duyen-codes
Submitted
One thing remained unsolved. When I scale down the viewport from desktop view, there's a line on the right side of the grid container. I couldn't figure out why and how to fix that. I'd be thankful if you can look at my codes and give me a hint on where and how to get rid of that line.
Rafal• 1,395@grizhlieCodes
Posted
At first i thought the fix was going to be
widthorpaddingbased but after a few thoughts I decided that the solution is to simply supply yourmainwith amax-width. The card shouldn't get that small. There's an issue with the content being squeezed too much and it messes up the width of the card for some reason.For the mobile view you had the right idea,
width: 85%. I suggest we keep it that way and remove thatwidth: 40%from the media query.On that note, a few suggestions:
Assuming you'll remove the
40%:Overall the
width: 40%should be avoided UNLESS it's coupled with one more step:Add
max-widthsper display size. In this case you can just add a max-width for the mobile view, I went with350pxfor this and then635pxfor larger display sizes. You can tweak these to your liking, the point is to control this a bit more whilst leaving it mostly responsive with that %.I think you were trying to get that
width: x%do too much work by itself basically. We can help it out withmax-width. This is generally an awesome strategy for a ton of stuff you will code.Best part about it is that this is a tiny improvement and overall you've done a great job 👍
Marked as helpful
0 - Lesley Kimutai• 565
@Leskim
Submitted
The hover part of the top image .... The view image blends into the box image which is not as per the design. Its minor but any help on how I would have tackled that part better will be highly appreciated.
Thanks in advance 🤜🏽
Rafal• 1,395@grizhlieCodes
Posted
Hi Lesley!
Overall I think this is a great job. I could write a few pointers but I thought the best thing to do would be to show you how to best tackle this:
A video i actually posted yesterday covering this solution. The exact part you're interested in I think would be found in this timestamp.
Otherwise feel free to watch the whole thing and see if you get any ideas. I used SCSS but it's basically CSS with nesting, anything you're unsure about just let me know and i'll type up a juicy/detailed response!
Alternatively I created this codepen demo for you, it's the exact solution 'required' or one I would suggest anyway for this particular problem.
If you have any questions just let me know.
Marked as helpful
0 - Rafal• 1,395
@grizhlieCodes
Submitted
Just another vanilla solution I did for fun, really love the simple design, reminds me of Tailwind with the color-scheme. I wanted to write it up to help share some CSS concepts with a few folks in FEM. Also ended up recording it so if anyone is interested in my thinking as I coded feel free to have a looksie.
Rafal• 1,395@grizhlieCodes
Posted
I felt the fonts/text was off btw but I didn't investigate deeply enough as to the differences. Sorry 😅
0 - Manjiri Phatak• 90
@manjiriphatak
Submitted
Hello everyone! Please go through my code and let me know of any improvements that I can work on. it took me 4 hours to get this done...is it extremely slow? I would like to know if
- My css is well structured?
- media queries are good or not
Rafal• 1,395@grizhlieCodes
Posted
Hiya,
Don't worry about 'slow' and 'fast', especially if you're starting out. You will improve with speed the more you code, and if you initially take your time than just do that and learn as much as possible in each project.
As for feedback:
I would structure this a bit differently. You can create 2 divs (sections) within the container for the image and the text.
<div class="container"> <div class="img-container"> </div> <div class="text-container"> </div> </div>This already allows you for a bit more control imo.
You can then follow up by adding padding to the text-container. I saw that you pushed down the
.containerby addingpadding-bottomto yourcancel-order-section. This isn't necessarily the best practice.Padding should be used by a parent to style the content inside of itself. Margin is used to interact with other elements around it.
So if we use padding to add space to the text-container it would make more sense.
.text-container { padding: 32px; }Everything in text container will now be positioned a bit better.
We can then take care of the other margins you have. Instead of applying them to every other/single element we can do a slightly more systemic approach.
.text-container { display: flex; flex-flow: column nowrap; justify-content: start; align-items: center; gap: 16px; }Now our elements have at least
16pxof gap between them. You don't need to set this for every single element, instead just set it for the container (why I suggested the approach initially tbh).Whilst looking at the design the next thing I spotted is that the first element, heading, has
16pxof gap, but the other elements actually have24pxbetween them. We can fix this easily:.text-container > *:not(:nth-child(1)) { margin-bottom: 8px; }So now an additional of 8px will be added to
margin-bottomto every element except heading.I have more tips etc but I decided to make a video on it instead of just typing away here. I'll message you when I've uploaded the video. It should be relatively helpful and should explain the above concepts a bit more easily, especially if you could see all the code.
I have videos on other projects that I cover 'best practices' in. Feel free to check them out as they are frontend mentor projects :)
- Stats Preview Component
- Profile Card Component
- This one is a bit more complicated as I assume some knowledge and I use SASS. NFT Preview Card
1 - Rafal• 1,395
@grizhlieCodes
Submitted
Just thought I'd make a video covering this for no particular reason.
Think I could have set the heading as a link, that was a mistake that I haven't gone back to correct yet but not a massive deal to me.
If you have any interesting feedback, let me know 🙂
Cheers!
Rafal• 1,395@grizhlieCodes
Posted
Oops. Correct link.
0 - Hayd• 270
@haydee75
Submitted
Hi! :) That challenge was interesting and challenging! Does anyone know a good way to avoid using masonry and have the same behaviour ? The documentation I found on internet wasnt that great
Rafal• 1,395@grizhlieCodes
Posted
Just posting a portion of the code separately, portion because I think I removed some CSS and some fine-tuning to the below JS, don't ask me why... The idea was for this code to run when the page was mounted in the browser (I was using Svelte) and position the images in the grid by telling each image how many rows it should span. Then
grid-auto-flow: dense;would take care of the rest, because css-grid is magic.const updateGrid = (gridItems) => { gridItems.forEach((item) => { const image = item.querySelector('img'); const rowHeight = 20; //js const gap = 20; // js // height of element before any manipulation const itemHeight = image.naturalHeight; const rowSpan = Math.ceil((itemHeight + gap) / (rowHeight + gap)); item.style.gridRowEnd = 'span ' + rowSpan; const articleHeight = (rowHeight + gap) * (rowSpan - 1); item.style.height = articleHeight + 'px'; }); }; let mounted = false; onMount(() => { mounted = true; const items = document.querySelectorAll('.art-piece'); // updateGrid(items); });Auto-flow was set to dense of course and all that good stuff that grid offers. Also learned about the existence of
naturalHeightin javascript, pretty useful stuff.Marked as helpful
0 - Hayd• 270
@haydee75
Submitted
Hi! :) That challenge was interesting and challenging! Does anyone know a good way to avoid using masonry and have the same behaviour ? The documentation I found on internet wasnt that great
Rafal• 1,395@grizhlieCodes
Posted
Solution looks great 😁!
As for avoiding using masonry, I was pondering the same. I jumped on this one and I think I finished it first but after a little trial and error I realised it would take a ton of time to get an algorithm in place. I ended up folding and applying 4 columns. And I dislike using libraries for this sort of stuff as I end up learning less so for now I'm avoiding them.
The concept however is simple. Application of it gets slightly complicated around step 4 (succeeding step 4 was the closest I got to) and rather complicated around step 5.
- You make the container into CSS grid.
- You set the columns as you see fit, assuming some repeat and auto-fit.
- The rows is where the 'magic' starts to take place. You set them to something like
grid-auto-rows: 20px. - Then we get into Javascript. We basically want to get the height of each image and apply
grid-row: span x. The math behind this means that we need to include how many rows the image spans + count in the gap-heights. It gets rather messy from this point on. - Because now we need to write an algorithm that best tries to create a grid that has columns as similar in height as possible. The most efficient way I think would be to order the images by height. Tallest images would be fitted as the first image in each column. Then you keep on applying some sort of logic that seems to work. I suck at math so I can't even imagine it, i'd need to trial-and-error.
But that is the basic gist. I got until step 4. So it was fully functioning but I couldn't get the images to line-up. Something would always be sticking out. By using my own JS i got a design that is more or less the same as yours, 1 image was just sticking out on the bottom. Overall it worked but wasn't finished so I scrapped it for now 😅.
Marked as helpful
0 - Azmi Fitra• 95
@azmifitra
Submitted
If there any feedbacks for me, please let me know. Thanks!
Rafal• 1,395@grizhlieCodes
Posted
Hi Azmi, thanks for sharing your solution! I'm not great at giving praise, WIP, but overall I think you've shown that you can figure out how to solve a problem, turning design into code. I'll take a bit of time trying to share concepts of how you can solve it better, hope it's somewhat useful 😅. I tend to overshare, sorry in advance.
Height and avoiding pixel-perfection
Height is an odd one. I don't recommend using height for anything as it leads to an older way of thinking about CSS, when 'pixel-perfect' sizing was sought. As time passed a very generic screen most people had became a big question mark for the developer as screens started deafferenting in sizes, browsers were added and different browsers supported different things and a website all of a sudden had to fit a 49" display as well as an iPhone SE.
So why should height be avoided? Imagine if you have a paragraph with content and you set the height to 50px. As soon as your width runs out because the screen is too small the content (text) will overflow. This break the user experience and makes us look bad, so lets avoid that.
Instead, if you really want to use height use
min-heightinstead. Min height will guarantee that your containers are responsive to the content they have if the screen gets too small for their particular widths. It could be an element with a min-width of 200px or 600px, it doesn't matter.Ultimately the point is to let the content dictate the size of the container. Because then it's responsive.
*I'm sharing this video not to promote anything but to share concepts (I made this video for another FEM 'student'): have a watch if you're interested in how we can achieve this design, almost pixel perfect, without setting heights: link. *
I'd love to do a video outlining these fundamental concepts but I'm lacking time this month 😫
"Containing" Content
This should be a simple one. We basically want to structure our HTML in a similar way as the design suggests. This is open for interpretation of course but lets see if we can agree on something:
The card (
.container) has 2 parts. Top and bottom and in tablet/desktop left and right. We can go 1 step further and call these 'sections' of the card.One part is
.info-contentand the other could be.img-container.Our grid becomes simpler then. We either have 2 rows and 1 column in mobile or 2 columns and 1 row in tablet +.
Our text-content becomes a simple flex-container that is styled to be a column.
Would you agree that this is a simpler approach or do you think I'm adding complexity? I personally think it's simpler. It's easier to focus on 1 part of code (lets say
.info-content) instead of looking at the various individual elements within our grid. The code would look something like this:<div class="container"> <section class="info-content"> <h1 class="info-content__heading">...</h1> <p class="info-content__key-points">Discover the bene...........</p> /* remaining code */ </section> <section class="img-container"> <img ....> </section> </div>Here I'm just showing an example of how you can structure this. You might have noticed I used
<h1>, <p>instead of<span>. This is because<span>is used to 'style' text. It is not meant to be used for every single text-element you have. A more typical approach to use span is this:<p> I am a paragraph! <span>I am some styled text that looks different but is a part of the paragraph! </span> </p>My approach of course makes your grid infinitely simpler.
.container { grid: "img" min-content "info" min-content / 1fr; /* you can read the above as : grid: "grid-template-area" row-height (row1) "grid-template-area" row-height (row2) / grid-template-columns; */ }And of course do similar for tablet/desktop.
I have a ton of other suggestions but perhaps, PERHAPS, you can watch the video of my solution and just observe some things I did differently. Or just ask other questions here "how can I..." etc.
If you have any questions I'm happy to respond to them. Here or on Youtube, or w/e works for you. I'm running out of time and I could continue for hours as I'm sort of fascinated with the most efficient ways of achieving things whilst leaving all code very readable.
Marked as helpful
5 @Rezzak48
Submitted
Any Feedback? t
Rafal• 1,395@grizhlieCodes
Posted
Again, looking pretty strong! I'll probably nit-pick and will focus on concepts more than actual code.
Lets go with a list?
- **Avoid using height. If you want to use
heightI recommendmin-height. **
Why? GREAT QUESTION! The concept here is simple: we want to let the content decide how large a container is. When we write CSS we are working with alot of unknowns: What is the screen size of the user's client? What browser and what version are they using? Do they prefer darkmode or default? Etc.
This gets into the core of FRONT END web development: we don't want to create pixel-perfect styling for the content. We want to create adaptable content that is responsive. You actually did really well in this regard, but like all of us a few improvements here and there will just carry you further!
Back to the height.
Lets say, for whatever reason, our user's display is
200px wide. And lets say your.reviewcard is responsive in width. The content will not fit in the.reviewcontainer! You'll see the.star-icon's breaking out of the container on the top. Why?height: 62px(you should be using 6.2rem btw, keep it consistent 😁)! The height will break it. Instead you could usemin-height: 6.2remand voila. Now the content is responsive on ultra-thin displays.Lesson: Don't use height as we simply don't know the users display etc. Allow content to rule the height and allow the browser to recalculate everything for you.
This is mega important I think so ask me questions if I failed to explain it.
- What if the user, for whatever reason, is using a very wide display, lets say 34"+?
Another responsiveness lesson: the background image you are using will be tucked away in the depths of the far-left-top corner of the users display. I have a 49" display and so it's very simple for me to test this. You could test it by zooming out whilst building your projects.
The fix is rather simple too, one line of code to
body:body { background-position: right 66vw top 0%; }This should be un-intuitive. If we want the background to be in the
leftwhy are we usingright?? I've no clue how the math works unfortunately. But this keeps the image relatively 'centred'. If you tried doing this withleftit would mess it up and it would at some point be located outside of the viewport. Depths of CSS I've not yet covered...- Be brave with UI
I'm really only talking about you being comfortable at judging when something is too small/large and just making an 'executive-decision' to change it.
Lets take the
.reviewcard again. I think that during the mobile -> desktop transition it becomes too wide, taking up WAY more space than it needs to.- Continuing point 3 but introducing clamp
Lets use clamp for this to declare a
min,optimalandmaxwidths:.review { /* other code*/ width: width: clamp(22rem, 50vw, 41rem); }I'm pretty sure if you removed ALL other widths associated with
.reviewthis code would be sufficient. To understand clamp check this video out.You can use clamp for anything tbh. Font size, width, padding, margin, name it. It allows you to easily write responsive measurements depending on a the screen width.
Once you have played around with clamp you can use this tool I built to get the EXACT clamp code you need. Don't use it now, it wouldn't make much sense (yet). I'm not brilliant with math so the math I took from someone else 😅.
If you use SASS then you can use this SCSS code as a function. If you don't use SASS yet, don't worry about it.
$breakpoints-units: ( "mobile": 375, "mobile-wide": 480, "phablet": 560, "tablet-small": 640, "tablet": 768, "tablet-wide": 1024, "desktop": 1248, "desktop-wide": 1440 ); @function fluid($size, $minSize, $maxSize, $oneRem: 10){ @if map-has-key($breakpoints-units, $size){ $size: map-get($breakpoints-units, $size); $mobile: map-get($breakpoints-units, mobile); $minWidth: $mobile / $oneRem; $maxWidth: $size / $oneRem; $slope: ($maxSize - $minSize) / ($maxWidth - $minWidth); $yAxisIntersection: -$minWidth * $slope + $minSize; $vwUnits: $slope * 100; @return clamp(#{$minSize}rem, #{$yAxisIntersection}rem + #{$vwUnits}vw,#{$maxSize}rem); } }As an example of what the above code would do:
/* If we type this in SCSS /* div { padding: fluid(tablet, 2, 4); } /* it will output the following CSS */ div { padding: clamp(2rem, 1.0458015267rem + 2.5445292621vw, 3rem); }So our padding will be a minimum of 2rem (starting at around 375px). And it will grow with every pixel of increased viewport-width until it reaches 3rem.
Magic 🧝♀️✨.
I think that's enough. I could blab on but it'd be less and less useful lol. Hope you get something out of this.
- Avoid setting
height, setmin-heightif you must. - Think of very small AND very large displays before you finish a project and implement tiny styling fixes.
- Challenge designed/your own UI. As an obvious example, a single word with
font-size: 1remshouldn't havepadding: 10rem. - Clamp baby!!! ✨🤩
Edit
Oh yeah. I noticed you were using
max-widthfor your media queries. Generally this means you designed the Desktop version of the page first, is this correct?Either way, the best practice is to code the mobile version first and use
min-widthfor media queries.The logic is quite simple too. A mobile page is easier to code and it gives your a MVP (minimum viable product). It's usually a simple, single column, layout that you can then easily style etc.
As you increase in screen size, you have more real-estate to work with. Then we add complexity, you see this in designs often I'm sure. Things get more interesting for desktop sizes.
And generally speaking it's easier to do the SIMPLE first and then add complexity. Because if we start with desktop, we are starting with complexity and then are trying to simplify it, this is often much more difficult than the alternative approach.
1- **Avoid using height. If you want to use
@Rezzak48
Submitted
This challenge proved to me again that using rem is really helpful more than using px unit.
Any Feedback?!😊
Rafal• 1,395@grizhlieCodes
Posted
Howdy, great job for this solution.
Overall, I don't have much to add, very clean and concise code that shows strong understanding of a lot of core css.
I did notice you added a
height: 13.75remto your image, there is no need. Also themax-height: 100%doesn't do anything. I'm pretty sure this is because the image was built for this width, so when it spreads towidth: 100%the aspect ratio that calculates the height of the image is already at that13.75rem. This probably means thatobject-fitwon't do much here either since the image already fits perfectly to that width.Oh yeah, I saw rather strange amounts of margins of
0.9375remso I can only assume you haven't come across this glorious method::root { font-size: 62.5%; }Now if you type
1.5remit will equal to15px.1rem = 10px, 0.1rem = 1pxand so on. This is easier to work with, not sure if there's a benefit to doing all that 'math', lets let the browser handle all that work instead!The logic is simple:
100/16*10 = 62.5. We basically want to calculate the % of a rem unit that we need in order to get1rem = 10pxand this is what it all does.So now you can crack on and write
margin: 1.5rem😁Perhaps a hover-effect for the main button would be nice (
payment-button)? Even changingopacity: 0.7or something as it's usually quickest to implement.Ah, I had a look at your button afterwards and I noticed that you have:
<button> <a href>Proceed to Payment</a> </button>Generally speaking you either want a link or a button. Having both makes little sense and might mess up the semantics a bit. An anchor can be styled like a button. An anchor can be given Javascript functionality that allows it to pass data to somewhere else whilst using the link.
Oh ya - this is actually 'wrong' html I think. I made this mistake a week ago so it's still fresh:
The interactive element [a] must not appear as a descendant of the button element.linkHope this was somewhat useful!
Marked as helpful
2
