No specific questions, but as always feel free to suggest improvements or question why I've done things the way I have.
I would usually create spacer variables for use on things like font-sizes and paddings, but didn't for this one as it's so small and as I don't have the original design files. This means I am just guessing on sizes - hence random 'magic numbers' in the scss
Great job Grace. I really enjoyed how thorough you were with accessibility. The H1 was a nice touch. I did find some things that I'd like to highlight:
BEM naming. There looks to be nested block names inside elements. EX: c-showcase__card-titleEM units in c-showcase__card-btn when you already have REM set to 16px as the default. This will be a problem if you every nested this component.And now for the nit picky items:
opacity of 0.9 right now, but the design looks much lighter.680px. I'm used to using a common standard breakpoint < 768px which will accommodate for all mobile/tablet screens from 0 to 767px.Hey Natalia. This looks pretty good, but I did notice some details that could improve your solution:
card__component could be a block as card-component. Then anything that is an Element could follow.I tried to use as less code as possible to not clutter it or use more lines then needed. Are there any ways i could have made my code cleaner?
Thanks in advance
Hey Richard. I can spot some was you can make this clean cleaner. Here's a list:
div to CSS selectors. This makes it too specific and harder to override. Simple is best.My solution seems similar to the designs, but don´t know, if the code is well-organized and what could I improve. Grateful for any suggestions/critics/feedback!
Looks really close to the design Anna. The colors look great and the overall implementation is clean. Here's some nit picky things I saw that can improve:
<body>. This is making the window taller than it should be.min-width instead, it will be a mobile first approach. https://getbootstrap.com/docs/5.1/layout/breakpoints/border: 2px solid transparent; to the regular state. This way when the hover state comes into play, it doesn't add an additional 4 pixels to the height and width making the elements shift.This is the longest project i have done, so far. Lot of Learning and unlearning. This was done desktop first (which i later realised was a mistake)
comments:
Still a work in progress, yet to completely fix accesibilty.
Cooordinating the modals was a bit of a challenge, that is why the js is messy.
Still on working on focusing on the modal the specific pledge selected.
working on using localstorage to persist the state.
comments and corrections are welcome on how I could have done better.
This is a great start Kevin. It definitely looks the part, but can use some touchup. Knowing how particular designers and some companies ware with being pixel perfect, here are some suggestions:
Download an image overlay plugin like PixelParrallel. It allows you to see where you're a little off in certain areas based on the design mock ups.
Modals background should be darker.
Modal options should allow the user to click the entire block to select the option instead of the radio button. This will increase UX and conversion.
You did a great job centering the modals!
Mobile modal starts in the center of the modal and not the top.
Mobile font size and line-height seems off. Definitely the main__wrapper.
Mobile menu should have a more subtle transition on open/close.
Mobile menu <a> should expand the entire width for it's click state. Increases UX/UI.
Make sure logo links to homepage.
Instead of using media queries to adjust the mobile screen, build your CSS mobile first. On a slow mobile phone, you do not want the user's device to work twice as hard to render styles. But on Desktop, the user is most likely using a fast internet connection and can handle the additional media queries.
Hi everyone! Happy to be back and submitting another challenge! I started this project to practice React and concepts I’ve been learning from Josh Comeau’s course. I soon realized I was writing the same code for every planet, since every page looks the same, and I thought there MUST be a better way. After some research - enter Nextjs. The dynamic paths seem like the solution I was looking for. That also meant a huge refactor of the entire project. 🤯🤷🏻♀️
My process, struggles, and bugs:
The site flickers on mobile so I only enabled the keyframes on desktop.
I didn’t try to contain the content into a max-width.I think it still looks ok on a 2k width screen.
It was difficult to work around Nextjs’ Image component since the actual <img> tag is nested under two divs with inline-styling. As such, I don’t know for sure if the images are bulletproof. Please let me know if you find any overflow or other quirks with the images.
I’m NOT an animator at all lol but I wanted to learn tooling so I tried to work with Framer Motion. It’s a great library but in some instances, it inserts its own transform property which overrides my CSS. I’ve had to modify some of my styles to accommodate, specifically on the images where I was initially using scale to change dimensions. I’d love to know how to also animate the planets when you click on the tabs, aside from animating when the page changes. The library requires a single unique key for the transition, and I’m already passing one for the page change. What’s a different approach to also animate for the tab clicks?
How do i make it so that ‘/‘ and ‘/earth’ are both showing the same content? Right now I’m passing the Earth prop to the index page.
How do you pass an either-or props to styled components? This code works, but I wonder about bugs.
Please share your thoughts! Had a great time working on the CSS, the navbar design was a new challenge for me. There’s still some more to do like the readme, cleaning up repeating code, and html warnings but I’ve sat on this “finished” project far too long to not get feedback which resulted to a lot of unnecessary frustration on my end. And that is the big takeaway.
I'm just going to review the design and not the React code. Here's some feedback:
• Instead of showing nothing as the homepage, choose a default and show that. Without a default, it makes the user feel like there's an error with the site.
• The level of detail you put into the responsiveness is fantastic!
• I don't see the benefit of using CSS Grid in the Header, but it looks good.
• Instead of using margin's in the Stats_Item, use gap: 0.6875rem on Stats_Line.
• For the main nav, I'd expand the click area to the entire header. It will increase the UX and less chances of the user not being able to click on the menu item.
• The plant transitions could be better when navigating through overview, structure, and geology.
Overall, the site works really well. Great job!
if would like give me feedback in my code, be welcome.
Don't be discouraged by this, but let it be a way for you to learn. I'm particular with design reviews because you're going to get grind out by the designer for not paying attention to the details. I hope I'm not too harsh, and let me know if none of this makes sense. Cheers!
Items to improve on:
The details. You may not have the Sketch file, but if you overlay the design onto your implement or looked at it side by side, you'll see some differences.
On Desktop View • Border-radius on the container is off • The content title and body should be left justified • Do not use <strong> unless it's made to be bold or stand out. For this instance, it's misleading because the entire title should be prominent. • Check the font sizes. They look a few sizes off on the title. • The stats column should be split into 3 equal parts and aligned to the start and end of the container. • The stats column label font and font size are incorrect. • The image should have an overlay fade. Right now it's way too bright.
Responsiveness • If you look at your implementation at 900px to 1400px, the image is cut off. Instead, make the two sections equal in size and I'd start the mobile view below 768px.
On Mobile view • The design should work on the minimum screen size of 375px wide. Why? Because this is the standard iPhone screen size nowadays. • All text should be aligned centered on mobile. • Please check the padding from the title to the body, from the body to the stats columns, and the stats columns to the bottom. It's all inconsistent and doesn't match the design.
I got it working but backwards.... I either have it to where none of the listings will show until a filter is selected, then the filtered jobs show up. OR all listings show up and the filtered jobs are REMOVED instead of the not-matching jobs from disappearing. Not sure how to get it to go the other way but it's pretty much done, slightly functional lol
Good work here Kele. I know this challenge takes time to style and implement. There are some things you should clean up, but you should be proud of how far you've gotten.
active-states.jpg, it tells you what should be clickable. Currently, you cannot click on the individual job posting tags. That's how a user will filter the results.Rate my design please, Any feedback and suggestions on how I can improve are very welcome!
Hey Esraa. You're getting better at implementing designs, but I do see some areas that you can improve on:
max-width: 920px.margin: auto;. Because you're already using flexbox, add place-items: center;. This will center the elements vertically and horizontally.Change logs:
v2.0: Still have to fix flex boxes so they aren't a static size.
v2.1:
v2.2:
Hey Cara. Looks like you were able to redo this using Flexbox. Great job! This implementation looks very much like the design too. I did find some areas that can improve:
.main should be 920px wide and the child containers should span the entire 920 width..left-container. Allow this section to be fluid. This way if anything changed, it's easier to know where the fault is. If you inspect this element with browser dev tools, you'll see that the margin cuts the icons in half. Its true size is lost.<div class="icon-bg"> has a hover state, but it's true clickable region is the <img>. For better usability, remove the div and move .icon-bg to <img class="icon-bg">. Less DOM elements, cleaner code, win-win..progress-bar-active should be 81.5% of the width.Hey Jisha! You produced a good solution, however I found some things that can improved with your implementation:
font-weight: 500 when they should be 700.border-radius and box-shadow is missing.border-radius and box-shadow are incorrect as well.width: 45% is not ideal. It should be 100% and add margin to the left/right to space the content from the edges.Hey Shannon. You did a good job including all the elements and styles. I did find some items that can improve:
box-shadow from the main container and the submit button.