Skip to content

Comments

MVP & Stretch Goals#1

Open
ajb85 wants to merge 2 commits intomasterfrom
andrew-brush
Open

MVP & Stretch Goals#1
ajb85 wants to merge 2 commits intomasterfrom
andrew-brush

Conversation

@ajb85
Copy link
Owner

@ajb85 ajb85 commented Jan 16, 2019

Made an effort to get some more comments in.

Code should meet all MVP and stretch requirements. Layout still looks off, though. Will continue to work on it.

Also have it hosted at https://ajbrush.com/resume/index.html

ajb85 added 2 commits January 16, 2019 18:28
…ds to be removed and more style to the resume
…ered. Removed title left-margin in mobile view
@ajb85 ajb85 requested a review from GannonDetroit January 16, 2019 23:50
Copy link
Collaborator

@GannonDetroit GannonDetroit left a comment

Choose a reason for hiding this comment

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

Your project looks great on the screen. Your code is well written and very organized. You nailed the MVP and hit the stretch. Just go back and add the missing meta tag and you're good to go.

I look forward to see what you add to this as you continue to go through your training at Lambda.


<title>My Resume</title>

<link href="https://fonts.googleapis.com/css?family=Noto+Serif" rel="stylesheet">
Copy link
Collaborator

Choose a reason for hiding this comment

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

good job bringing in the google fonts, but it looks like you're missing a certain meta tag that should be around this area. Let me know when you add it in.

</header>
<main>

<!-- Summary of Skills -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

great job with adding the white space and comments in the code.

b, u, i, center,
dl, dt, dd, ol, ul, li,
fieldset, form, label, legend,
table, caption, tbody, tfoot, thead, tr, th, td,
Copy link
Collaborator

Choose a reason for hiding this comment

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

What happened here?

Did you delete and have to re-add the reset?


html, body {
height: 100%;
@secondary: #1b0e3a; //dark black --> purple
Copy link
Collaborator

Choose a reason for hiding this comment

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

good job using variables. I like that you actually used hex codes and not just "black" or "blue". You even added a comment like in line 158 to explain what the hex color code should roughly look like. Great job

@accent: #fffbaa; // light yellow

// Mixins
.flexAlign(@justify, @align) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

good job on the mixins, you've demonstrated you understand how and why to use them. These look like quality Mixin's as well.

Good job.

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