Kathryn• 320
@KathrynDavies123
Submitted
- Making this site responsive felt really janky, wondering if there is a more streamlined way to do this without making so many media queries?
- Are there any ways my code can be optimized further?
Joshua Mo
@joshua-mo-143
All comments
- Joshua Mo• 130
@joshua-mo-143
Posted
I would say try to make the website look as good as possible on mobile first. The workflow gets much quicker if you build for mobile then adjust for larger screens later on.
Also, you should put your each of your links in the
ulelement in alielement so the HTML will recognise them as separate items in a list.Hope this helps!
Marked as helpful
1 Good challenge! If anyone knows how to add an email error message when pressing a button without JS I will be greatful! Also an easy way to change a color on SVG image. Thank you!
Joshua Mo• 130@joshua-mo-143
Posted
Hi there!
I don't really think there is any way to add an email error message without JS, but you can add validation to individual input boxes (regex patterns, input type, that kind of thing) to make sure that only certain requirements are met. A quick one I'd add is "required", just to ensure the user has actually inputted something in the newsletter subscription form.
0- Hyeon Park• 350
@hkparkjs
Submitted
Feedback is welcome!
Joshua Mo• 130@joshua-mo-143
Posted
I'd try putting a bit of transition duration on the hover animation with CSS. The component looks pretty good but I think having more of a smooth transition on the color change would make it that much better
1 - martin nkemakolam• 260
@martinnkemakolam
Submitted
I used useEffect to call a function on change of className of a project I did not so while back and also used it here. do you guys recommend this
Joshua Mo• 130@joshua-mo-143
Posted
Hey man, this looks pretty good. Might need a bit of UI tweaking on mobile but otherwise, at a base level it's decent.
I would personally avoid trying to use useEffect for state changes because you can end up in an infinite render loop if you're not careful re: dependencies. You could just instead use
onClick={handleChange}or something similar and then put the change code in handleChange. Theoretically speaking, if I were to do this I'd probably just add or delete the string to an array of strings on click and then filter the results based on whether or not the word exists within the string of categories.I'd highly recommend splitting your main page into components as it's a bit confusing and would help with enforcing single-role responsibilities of components. This is the exact kind of thing that React is extremely good at and it'll help you much later on. If you're worried about passing props, you can just simply pass state setters and getters as props into child components.
You don't need to set any external data to a state. The data is automatically immutable unless you make an actual file change and it doesn't appear that you make any direct changes to the data.
Hope this helps!
Marked as helpful
0 - SaynumB• 70
@boomtnt2843
Submitted
Joshua Mo• 130@joshua-mo-143
Posted
Hey there, this looks like a pretty good solution so far.
I think going forward, trying out CSS Grid to align the website layout would be a good idea to try and focus on. The site looks perfectly fine on desktop, but on mobile/tablet layout the top left part of the page (especially the main article splash text/button) becomes misaligned with the black sidebar menu which is a bit of a shame.
Otherwise it looks pretty good. Using article or section instead of div to help distinguish between articles/sections on the sitepage would help a lot with adding semantic HTML.
1 - Milosz Lewandowski• 130
@yerbaMatte
Submitted
Hi, what do you think?
Joshua Mo• 130@joshua-mo-143
Posted
Hi there!
Don't forget to change your README.md at some point, it looks like you've just copy and pasted the template in. Also, adding an id to things will help to identify individual elements.
The image looks broken but I think someone else has already provided a solution to that.
0
