Our reporter did not find any issues in this project! 🎉
Submitted
Loopstudios Landing Page With React
#react#sass/scss
P
ApplePieGiraffe• 30,105
@ApplePieGiraffe
Design comparison
Reports
Accessibility report (0)
ApplePieGiraffe’s questions for the community
Hello, everyone! 👋
This is my first challenge with React and as usual, I learned a lot! 😆
At first, I was pretty confused about how to organize the files in my project and I'm still unsure if my React is very clean in some places (since I just began learning React). My Sass turned out to be, well, kind of messy (and I'll keep in mind to use a better file structure next time because I simply created separate files for my components for this project). 😅
On the bright side, I created this pretty sweet hover effect for the cards in the "Creations" section from scratch, and I quite like it. 😊
Any feedback (especially on React) is welcome and appreciated! 😀
Happy coding! 😄
Community feedback
Michel Oliveira• 230
@oliveiring
Posted
hey, I did use display none and block and It works, but I don't if that correct.
1
P
ApplePieGiraffe• 30,105
@ApplePieGiraffe
Posted
@oliveiring
That might work, but I think using the picture element might be a little better for semantics and will allow you to determine when the image changes right in your HTML, which will probably make things a little easier to manage.
0
Michel Oliveira• 230
@oliveiring
Posted
how to replace the img desktop to mobile?
1
P
ApplePieGiraffe• 30,105
@ApplePieGiraffe
Posted
@oliveiring
Hey, Michel! 👋
I used the <picture> element to change between the desktop and mobile images for the cards in the "Our Creations" section. You can learn more about the picture element and how to use it here.
1
zhiwei• 55
@softmagnet
Posted
A bit late but i think you can use window.matchMedia for js media query instead of using listener in Header.js to constantly track the size of the window and set state every time it changes. I think it is less costly.
1
P
ApplePieGiraffe• 30,105
@ApplePieGiraffe
Posted
@softmagnet
Thanks for the tip! 😀
0
P
Elise Rousseau• 685
@SarahHenriette
Posted
Hi ApplePieGiraffe 😊 !
I'm just a fan of the hover effect you put on the images in the "our creations" section and also of the effect you put on the burger menu 😃👏
4
P
ApplePieGiraffe• 30,105
@ApplePieGiraffe
Posted
@SarahHenriette
Thanks, Sarah! 😊
3
Steven Toben• 750
@steventoben
Posted
For code quality I'd highly recommend code splitting. You're functional components are really large, generally a component should be broken down to the deepest level. It looks like you should break down Nav into it's own component, List, as well as list items. You should also use the mapping feature to map list items to a list so you don't have to repeat lines of code. It looks like you're using anchor elements and images enough to break them into their own component. On your hamburger menu the toggling state is overly complicated, as well as bad practice because state is async, so it could not be updated yet when that function is called. The best way to prevent that is just in your function writing setActive((prevState) => {return !prevState}); I'm pretty sure your useEffect hook is wrecking havoc on your performance, you have no dependency array so it's firing every single render, and useEffect is for managing side effects, so setting up the event listener to observe the window size is fine, but you must write a return statement that removes the event listener to clean up the effects. also add a dependency array to the hook containing window.innerWidth, that way it will only fire if that value changes. You could also consider debouncing since you really only want the value at the end of a resize. Also, you're not using props at all? props is one of the most important features in react and should be heavily incorporated in the app composition, instead of manually writing all of your data. Also one last thing, try to avoid styling like you did in the Creations functions. It looks nice but it hits performance heavy, especially an onMouseMove call, react will reconstruct all of the functions and variables at each render unless memoized or placed in a ref, so grabbing directly from the DOM over and over and repainting will cause some performance issues usually.
Onto the styles.... we've already talked too much about font-size lmao so let's just ignore that. In general your sass is pretty dirty. I'd recommend trying to compile your sass and take a look at the output of the css file, that would probably explain much better than I could. Your nesting is far too deep and often times unnecessary. Try to practice DRY and create a mixin for commonly used styles. If you have sass that's more than 3 levels deep there is something wrong with your method. Also it common practice to put media queries inside of selectors, not the other way around. As well as using em units for the media queries, also line-height should be unit-less, not measured in rem.
React and sass both have an atomic kind of methodology, so break down components as much as possible, use props and refs, it's ok to have hundreds of React components, and is actually quite common and really shows the beauty of React when you atomize everything.
Overall tho it looks good, you could try using css modules if you want to try a different method of styling, they're scoped to each component so you don't need to worry about clashing class names.
2
P
ApplePieGiraffe• 30,105
@ApplePieGiraffe
Posted
@steventoben
Wow, thanks. I really needed advice for React! I knew the logic for the mobile navigation menu was pretty bad and my Sass was quite ugly, but I didn't bother fixing those things (until now, I guess). 😀
To be clear, should I split my components into just separate functions or simply create new files for each component?
1
Steven Toben• 750
@steventoben
Posted
@ApplePieGiraffe No problem! If you need any React help I'm probably a good resource since I've been using it professionally for like 3 years so I have a pretty good grasp on most of it. It really depends on the situation, for example on this project here I'd make a Nav.jsx and a List.jsx but in that List component I'd make a ListItem function. If you did that you could also set List.Item = ListItem; Then export the List. Then when writing a list it would look like <List> <List.Item> List Item 1 </List.Item> </List> and that would let you write some cleaner code and keep both List and ListItem functions in the List file. It's ok to have multiple components in a file as long as they're tightly coupled, basically like helper components that wouldn't be used without the main component. Just like you'd never use an <li> element without a <ul>.
And yeah it's ok to use the nesting feature and it's an amazing feature but people often go overboard and don't think about what that sass code will look like as css code, cause in the end it's going to be converted to css so nesting will create tons of code repetition. It's a great feature but just make sure to think about what the css compilation will look like.
1
P
ApplePieGiraffe• 30,105
@ApplePieGiraffe
Posted
@steventoben
Okay, cool! 👍
1
P
Anna Leigh• 5,135
@brasspetals
Posted
Ah, you beat me to this one! I’m just about finished, but I’m glad our effects are a bit different, so I don’t have to change it up, lol. 😄 Although I did have a double take moment where I thought I was accidentally reading my own code because I also used .header-hero-container. 😂
Great job, as always! 🙌 The hover effect on the creation cards is really fun with the background text and response to cursor movement. The page also responds really well, and I know how much a pain figuring out the medium/transition styles were - at least for me!
I know nothing about React, so I’ll nitpick at the design matching instead:
- The logo is too large on the mobile version, and is also missing from the mobile menu when open. I really like the hover effects on the mobile menu though!
- The
font-weightof the headings should be 300 - slightly too heavy at the moment. - The hover on the social icons should be “underlined” like the other nav links if you’re going for design matching.
- The border overlap in the hero that Em mentioned starts at 340px and happens for about 10px or so.
Speaking of React, how are you liking it? I’m going over JS in more depth right now, but that was potentially next on my list due to the demand for it.
2
P
ApplePieGiraffe• 30,105
@ApplePieGiraffe
Posted
@brasspetals
I knew I was going to forget a couple of small details because I was focusing on React, but you have a good eye, of course, and so thanks for pointing out those things!
I decided to scrap the underline on hover the social media icons because I liked simply darkening them better. I actually had some other custom ideas for the hover states of those social media icons, but I didn't like the way things were going when I tried them out, so I scrapped them, too. 😅
Funny thing, I only included the 300 font-weight for Josephin Sans when linking my typefaces from Google fonts, so I didn't bother about explicitly setting a font-weight for the headings, but the browser must store that typeface by default or something because those headings are thick!
And I changed the z-index of a few elements to get the mobile menu working and apparently forgot all about the logo in the mobile layout!
React is good, and I like it because it utilizes vanilla JS constructs to accomplish things a little more than other frameworks do (apparently). TBH, I like Svelte better so far because it just makes so much sense (especially if you jump into it after HTML, CSS, and JS), but I have much to learn, as well, so we'll see how it goes!
Thanks! 😁
2
P
Anna Leigh• 5,135
@brasspetals
Posted
@ApplePieGiraffe I figured the icons were a conscious choice, but thought I'd toss it in there just in case. I had the same font-weight issue when I initially submitted my Insure project. Had to go back and tweak a bunch of layout elements. 🤦♀️ Never again.
Thanks for getting back to me about React (and Svelte) - good to know!
As always - happy coding, APG! 😄
1
P
ApplePieGiraffe• 30,105
@ApplePieGiraffe
Posted
@brasspetals
I just pushed a bunch of those fixes to the live site—so things should be good, now!
I can't wait to see your solution to this challenge! Let me know when you submit it! 😉
And, yes, of course—happy coding! 😁
2
shahab shafie• 70
@Shhb-Coder-1999
Posted
AWSOME WORK !!!!!🙌
1
P
ApplePieGiraffe• 30,105
@ApplePieGiraffe
Posted
@shahabs789
TY!
1
Aaron Stroud• 435
@astroud
Posted
Very nice 🍎🥧🦒!
1
P
ApplePieGiraffe• 30,105
@ApplePieGiraffe
Posted
@astroud
TY! 😀
2
P
Elie Bissor• 155
@ElieB77
Posted
Awesome solution !👌👌
1
P
ApplePieGiraffe• 30,105
@ApplePieGiraffe
Posted
@ElieB77
TY! 😀
1
I absolutely love that mobile nav effect! Utterly beautiful
1
I hardly touch react, but one of the most common issues I see with react projects is forgetting some of that base html knowledge picked in earlier challenges. That hamburger is a classic example I'm afraid - a div used instead of a button. You know that's gonna make me sad 😉
But I also know you have the power to fix it ☺
1
P
ApplePieGiraffe• 30,105
@ApplePieGiraffe
Posted
@grace-snow
Snap! 🤦♂️ I had been careful to use the button element for past hamburger menus I made but I completely forgot about it in this challenge! Thanks for pointing that out! 👍
1
Tesla_Ambassador• 2,900
@tesla-ambassador
Posted
This is by far the best loopstudios I have see!!! Cheers to Apple Pie Giraffe. I am in love with the hover effect! Take us to mars bro.
1
P
ApplePieGiraffe• 30,105
@ApplePieGiraffe
Posted
@tesla-ambassador
Thanks, dude! Can't wait to see yours! 😉
1
P
Emmilie Estabillo• 5,420
@emestabillo
Posted
Hey APG! Great work! The nav transition is pure silk lol and the hover on the creations section is really creative wow!
I’m viewing this on mobile and I’m getting a small bug where ‘Experiences’ in the header is touching the border of the div. It may just be on ios though, checked with Safari and Chrome.
Otherwise, this looks pretty much on point as expected 😀
1
P
ApplePieGiraffe• 30,105
@ApplePieGiraffe
Posted
@emestabillo
Thank you very much! 😊 I also just noticed that the word "experiences" is very close to the edge of the border around the heading. I think it's that way for all devices at that screen size, but it should be an easy fix. 👍
1
P
ApplePieGiraffe• 30,105
@ApplePieGiraffe
Posted
@emestabillo
BTW, I was going to mention that I actually took a bit of inspiration from you for this challenge (such as the responsiveness of the cards in the "Creations" section). 😉 And you should have seen my file structure before I learned a thing or two after looking at the folder structure of your most recent solution (that you made with React) earlier today—mine was a mess! 😂
1
P
Emmilie Estabillo• 5,420
@emestabillo
Posted
@ApplePieGiraffe Haha I deleted part of my first comment before I posted and it went: “I did the exact same thing to my sass files” 😂 I can’t say it’s best practice since I’m still learning but I’ve been studying multipager submissions on FEM and that seems to be one way of doing it. Thanks a lot for taking a look and commenting on my loop project 👍🏼
1
Please log in to post a comment
Log in with GitHub
Join our Slack community
Join over 180,000 people taking the challenges, talking about their code, helping each other, and chatting about all things front-end!