Skip to content

image resize #864

Open
malarc01 wants to merge 4 commits into
bloominstituteoftechnology:masterfrom
malarc01:master
Open

image resize #864
malarc01 wants to merge 4 commits into
bloominstituteoftechnology:masterfrom
malarc01:master

Conversation

@malarc01

Copy link
Copy Markdown

image resize

@GannonDetroit GannonDetroit left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Melki,

I like the increased about of comments you have in your commit history, great job on that!

I also LOVE that you're adding comments in your HTML code so it's far more professional and easier to read/get into. This is a habit I really want you to keep because when employers see that they will love you. It's a huge sign that you are team focused and think about other developers while you build your code. So great job with that.

I'm glad you removed the colored borders from this assignment, good job.

For some reason your navigation is different on this assignment than your previous assignments. The Great idea logo img is under the nav links and the font size is way smaller. I know you added the font-size 62.5% into the html tag on your CSS but the other side of that job is making the font 1.6rems. So you might need to go back in and add that to fix MVP.

You seem to be using your body tag in your css as your defacto container which might be causing some problems. (I did notice you add the max-width 880px requirement into the body which is why I'm assuming your using it as your container).

other than that you did a great of converting everything into percentages. You fixed all the issues I listed in your other assignment regarding the styling issues of mobile and tablet view with exception of the persistent missing bottom portion and right side portions (which can be resolved with the container I mentioned) So I'm glad you've demonstrated the ability to conquer most of the styling issues, just knock on the remaining errors and you'll be good. (only thing I didn't mention is tablet view for some reason has the opposite issue where we have a ton of extra white space at the bottom).

make the changes to the font sizes, the couple issues with mobile/tablet view, and the desktop nav bar/logo issue and you'll be good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants