Skip to content

finished Project#1

Merged
jskway merged 1 commit intomasterfrom
david-davis
Jan 19, 2020
Merged

finished Project#1
jskway merged 1 commit intomasterfrom
david-davis

Conversation

@dcdavis3920
Copy link
Copy Markdown
Owner

No description provided.

@dcdavis3920 dcdavis3920 requested a review from jskway January 16, 2020 09:14
@jskway
Copy link
Copy Markdown
Collaborator

jskway commented Jan 19, 2020

Great job grouping together your elements by sections, using the semantic <footer> and <section> tags, and applying styles to multiple CSS selectors to minimize repeated code.

Here are some improvements that could be made:

  • On line:20 (index.html) you could've used a <nav> here to be more semantic instead of a div
  • On line:127 (index.css) setting the padding to something like padding: 10px 2px instead of auto 2px would give the top of the page more space between the logo
  • On the design file, the nav links are grey and have the underline removed
  • On lines 197/198/279 (index.css), the bottom paddings and margins are set to 1400px.. The one on line 279 adds a bunch of extra whitespace to the bottom of your page
  • We discussed this in our 1:1 review (so I know you understand how to do it) - the Services, Product and Vision sections could use some more space above them to separate them from the middle-img
  • On line:63 (index.html) the content looks like an address.. what tag could you use other than <nav>?

Overall you made solid progress on this project. If you'd like to work on it some more and get a deeper understanding, here's a link to the solution code (https://repl.it/@irasanchez/User-Interface-3)

@jskway jskway merged commit 73a19fc into master Jan 19, 2020
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