Looking to hire developers?
@davidrhyne
@codycheshire95
Submitted
Someone wanna take a look at my code and give me some opinions on how I can shorten it, organize it better, and make it easier to understand. I would also like opinions on my readme.md and other files. This is my first time working with a repository, Netlify, and instructions. Thanks a lot!
@davidrhyne
Posted
Hi Cody,
Great job taking the first step with working with a repository and Netlify, may the be the first of many!
When working with CSS, try to put as many common settings with a parent class/element so that those settings can be reused by all those child classes/elements. And then any settings on the child classes with be those unique to those classes.
For example, with the btn classes, the following are repeated for each of the buttons.
display:inline-block;
cursor:pointer;
padding:16px 25px;
It would simplify your CSS to put those in a common btn class, positioned above all the other child btn classes, and then remove those common lines from all the child classes.
.btn {
border-radius:28px;
display:inline-block;
cursor:pointer;
padding:16px 25px;
}
.two {
background-color:hsl(0, 0%, 95%);
border:1px solid hsl(0, 0%, 95%);
color: hsl(184, 100%, 22%);
}
By all means, write the CSS as you are comfortable with. When you are done with a section, try to refactor the code by seeing what is being repeated often and if that repeated code can be put in one place vs on multiple elements.
One other CSS suggestion is to check the padding and margins. On the desktop view, there seems to be too much empty space below the buttons. And on the mobile view, it looks like there is too little padding below the button.
As for the the HTML, it is generally better practice to have only one H1 per page, but that is not set in stone. However, if possible, I would make multiple H1's into H2's or H3's.
Please keep up the good work and Happy Coding! David
Marked as helpful
@kyle4real
Submitted
This was a blast to build. How does my CSS organization look? Is my use of media queries good?
@davidrhyne
Posted
Hi Kyle,
It looks like you have a really good start with the project, so congratulations on the good start. The pages look really good at the mobile and desktop resolutions, pretty much pixel perfect, but I was noticing some word wrapping issues around the 800 and the 1000 pixel range in the client testimonial area.
Instead of using code breaks <br>, please consider using margin and padding on those containers so that they naturally resize themselves as the screen width changes. The data within the container will just break into new lines as necessary.
If it helps, I like to put a border around the various containers to see how they are sizing and interacting with the other containers.
Happy Coding! David
@SufyaanKhateeb
Submitted
How many media queries should one add to make the app responsive (approx.)?
@davidrhyne
Posted
Hi Sufyaan,
Disclaimer: I am an junior web developer, so please take this advice with that in mind.
Overall, I think it looks good. I am thinking this project should have at least 2 media queries because there is a great gap between 375 to 1440 pixels. I did mine around 600 and 825, but please use values that work for your HTML and CSS.
You may want to consider for following:
Keep up the good work! David
@davidrhyne
Submitted
This was my first solo project with the React and styled-components package, so I am sure it is not the prettiest code. But, I would like any suggestions for improving technique and/or better practices would be appreciated.
And I used styled-components because it is the one CSS package that I was most familiar with and it seemed to work out okay. Are there any recommendations for React styling packages?
Thank you!
@davidrhyne
Posted
I see that I forgot to include the alt tags with the images. I have that fixed but will wait a little while to push the update to make sure I do not break something during the push (still getting used to deploying on github).
edited: alt tags have been added and accessibility report is happy!
