-
Notifications
You must be signed in to change notification settings - Fork 566
Kevin Rodriguez -User-Interface-Project-Week #571
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overview
Kevin. Your overall html markup is well formed and clean. You have had a good go at the css for desktop and got it fairly close to the design file but it still needs work. your burger bar is in need of functionality and your nave needs some js. Your home page desktop view is a lot better than some that I have seen and is quite close to an mvp passing part but you need a little work on constraining your paragraphs and keeping your elements in check. Your navbar seems to work at certain resolutions which leads me to think that it needs constraints inside a container.
Breakdown
- Home Page Desktop Design (needs a little work almost mvp)
- navbar seems to work at certain desktop sizes
- home page mobile view (not done)
- Services desktop view (needs work)
- tabs functionality needs some work
Conclusions
Kevin. All in all I think you have done an OK job. the markup is clean but you still need to work on the css styles quite a bit to get it up to MVP complete. I think that you may want to go over some less / css tutorials over the holiday and resubmit it later to get MVP. your git workflow is ok too so keep up the good work Kevin 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functional Components - II
MVP
- Build layout HTML/LESS: Home page desktop design
- Build layout HTML/LESS: Home page mobile design
- Build layout HTML/LESS: Services page desktop design only
- Build layout HTML/LESS/JavaScript: Navigation system design (expanded and non)
- Build Custom Component HTML/LESS/JavaScript: Services page tab navigator design
What Looks Great
- This is really close to the design file
- The nav is functional! I know how much you fought that, good job!
- The tabs portion is spot on with design and function
- Frequent commits
- Good attempt at the stretch!
What Could Be Improved
- None of the mobile img files were used
- Commit messages should be descriptive to others and yourself.
- Code needs better formatting
What Could Be Added
- Add more comments so code is easier to remember or learn
Rating (1-3)
1 - this is bc the nav, while functional is not close enough in design to the file and the mobile doesn't follow the required use of mobile imgs. Those were included intentionally for a img swap challenge.
<img src="img/home/home-villas-img.png"> | ||
<h3 class="left">THE VILLAS</h3> | ||
<div class="p-words"> | ||
<p>The Villas bring to the table win-win survival strategies to ensure proactive domination. At the end of the day, going forward, a new normal that has evolved from generation X is on the runway heading towards a streamlined cloud solution.<br></p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the breaks are unnecessary
<footer> | ||
<span class="emailQ"><p>Interested in starting a project?<br> | ||
Let's talk:</p> | ||
<form class="email-css" action="/action_page.php"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what function does the action serve?
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you have this linked in your services page but it serves no function
@@ -0,0 +1,16 @@ | |||
const toggleMenu = () => menu.classList.toggle('menu--open'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good job making this its own component - It's clean
} | ||
} | ||
|
||
/* START HERE: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove the extraneous code and comments - these have no relevance to your code so it looks sloppy
justify-content: flex-start; | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
excellent job on global styles and understanding what should go here.
line-Height: 24px; | ||
padding: 0 0 0 10px; | ||
} | ||
.pic1{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't find a corresponding class in your html to match pic1 or pic 2?
</div> | ||
|
||
<div class="middle-content home-content hc2"> | ||
<img src="img/home/home-img-2.png"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
None of the mobile img files were used.
@VivaCode