-
Notifications
You must be signed in to change notification settings - Fork 954
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
Main #34
base: main
Are you sure you want to change the base?
Main #34
Conversation
Hello, I change the name of my repository because I needed to use the one with my nickname to use it as my README profile document. So mu personal portfolio lives now at https://andrygzt.github.io/andrygzt-personal-portfolio/ |
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.
Hi Andrea! Your submission has been scored as green. It was super fun going through your site! Loved all the sunsets, pictures of your adventures, and of course Lobo! You can find my comments in your code. Nice job! ✨
|
||
<h2><a class="links" href="https://goo.gl/maps/RScDR6n2FqFzuanu6">Bandon Beach</a></h2> | ||
<img class="images" src="images/home_sunset.JPG" alt="My favorite sunset 2" width="1000"> | ||
<br/> |
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.
Using the <br>
tag to control the spacing/positioning of elements is actually bad practice (and a common mistake). One of the major reasons to not use <br>
tags for positioning is because they affect the accessibility of the website. Instead, you should choose an appropriate tag for your element and then set the position using CSS.
Here's a good discussion on when to use <br>
tags: https://stackoverflow.com/questions/1726073/is-it-sometimes-bad-to-use-br
<h1 class="main"> <strong>Welcome</strong> </h1> | ||
|
||
|
||
<nav> |
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.
Great usage of semantic tags! ✨ Try to take full advantage of them when you can. Rather than having a <div>
tag nested inside of your <nav>
tag with the class, you could get rid of that <div>
and specify the class directly on the <nav>
tag.
<svg width="48px" height="48px" viewBox="0 0 48 48" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"> | ||
<title>Tiktok</title> | ||
<g stroke="none" stroke-width="1" fill="none" fill-rule="evenodd"> | ||
<path d="M38.0766847,15.8542954 C36.0693906,15.7935177 34.2504839,14.8341149 32.8791434,13.5466056 C32.1316475,12.8317108 31.540171,11.9694126 31.1415066,11.0151329 C30.7426093,10.0603874 30.5453728,9.03391952 30.5619062,8 L24.9731521,8 L24.9731521,28.8295196 C24.9731521,32.3434487 22.8773693,34.4182737 20.2765028,34.4182737 C19.6505623,34.4320127 19.0283477,34.3209362 18.4461858,34.0908659 C17.8640239,33.8612612 17.3337909,33.5175528 16.8862248,33.0797671 C16.4386588,32.6422142 16.0833071,32.1196657 15.8404292,31.5426268 C15.5977841,30.9658208 15.4727358,30.3459348 15.4727358,29.7202272 C15.4727358,29.0940539 15.5977841,28.4746337 15.8404292,27.8978277 C16.0833071,27.3207888 16.4386588,26.7980074 16.8862248,26.3604545 C17.3337909,25.9229017 17.8640239,25.5791933 18.4461858,25.3491229 C19.0283477,25.1192854 19.6505623,25.0084418 20.2765028,25.0219479 C20.7939283,25.0263724 21.3069293,25.1167239 21.794781,25.2902081 L21.794781,19.5985278 C21.2957518,19.4900128 20.7869423,19.436221 20.2765028,19.4380839 C18.2431278,19.4392483 16.2560928,20.0426009 14.5659604,21.1729264 C12.875828,22.303019 11.5587449,23.9090873 10.7814424,25.7878401 C10.003907,27.666593 9.80084889,29.7339663 10.1981162,31.7275214 C10.5953834,33.7217752 11.5748126,35.5530237 13.0129853,36.9904978 C14.4509252,38.4277391 16.2828722,39.4064696 18.277126,39.8028054 C20.2711469,40.1991413 22.3382874,39.9951517 24.2163416,39.2169177 C26.0948616,38.4384508 27.7002312,37.1209021 28.8296253,35.4300711 C29.9592522,33.7397058 30.5619062,31.7522051 30.5619062,29.7188301 L30.5619062,18.8324027 C32.7275484,20.3418321 35.3149087,21.0404263 38.0766847,21.0867664 L38.0766847,15.8542954 Z" id="Fill-1" fill="#000000"></path> |
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.
Super impressed that you used SVG for this! 🥳
</div> | ||
<h2>More favorite sunsets</h2> | ||
|
||
<div class="fling-minislide"> |
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.
Beautiful slides! Beautiful sunsets! Really great stuff here 💕
Only note - make sure your alt text is describing the image! This is important for accessibility.
</p> | ||
</section> | ||
|
||
<aside id="about-dog"> |
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.
Careful with your semantic html tags! This section isn't really an aside.
</ul> | ||
</section> | ||
<footer> | ||
<h4> © 2022 </h4> |
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.
Make sure you're only using <h1>-<h6>
tags for headings! They're used to automatically construct a table of contents.
You can read more about best practices here: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/Heading_Elements#usage_notes
<h1>My professional experience...</h1> | ||
|
||
<section class="carousel" aria-label="Gallery"> | ||
<ol class="carousel__viewport"> |
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.
Nice carousel! ✨
pointer-events: none; | ||
} | ||
|
||
.carousel::before { |
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.
Heads up that using ::before
to add content is discouraged due to accessibility concerns.
padding: 0; | ||
} | ||
|
||
.carousel { |
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.
Try not to have multiple style blocks for the same selector. To help with organization, each style block should have its own unique selector(s). So, I would recommend combining all your style blocks for .carousel
counter-increment: item; | ||
} | ||
|
||
.carousel__slide:nth-child(1) { |
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.
There's a lot of repeated code for the carousel in here. I would recommend finding ways to DRY your code a bit,
Personal Portfolio Site
Congratulations! You're submitting your assignment!
PLEASE NOTE. I request extra time to deliver this assignment because I had an accident with my right hand so I am working with one hand and mis progress is too slow. I notified about this to Jasmine Lopez and Sancha Elevado. Thank you so much!
Comprehension Questions