Skip to content

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

Open
wants to merge 39 commits into
base: master
Choose a base branch
from

Conversation

drkfogle
Copy link

Copy link

@decagondev decagondev left a 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 👍

Copy link

@VivaCode VivaCode left a 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>

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">

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?


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');

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:

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;
}
}
}

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{

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">

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.

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.

3 participants