Accelerate - Risha A. #10
Conversation
There was a problem hiding this comment.
Nice job.
I really like how clean the layout of your site is. I'd love to see you add some more content to this!
The parallax effect is really cool, and I'm glad you got that working. While the structure of the site doesn't technically meet the requirements (in terms of pages), it definitely meets the spirit of the requirements. Each of the sections is there, and the the portions of the sections that should have repeated elements for styling purposes (project and blog) do so and look great.
I'm really impressed that you decided to tackle a visually challenging layout like this, as it wouldn't have even occurred to me to try something like this! Keep playing with it and see what you can make it do! For me, that's the best way to get a feel for what certain CSS rules can do.
Structurally, I'm impressed how concise the markup is to enable this effect. Still, be sure to use validation tools to check the structure of your HTML, and don't pull in semantic tags simply for the sake of using semantic tags. Using plain divs and spans is perfectly reasonable when the structure is largely there for styling.
Accessibility-wise, I'm not sure how best to approach the background images. Arguably, here they are truly background (they don't have much if anything to do with the text or info on the page), but if you elected to use images that were more information rich, you might want to see how to adapt this layout so that the images could be placed in img tags with alt text.
Overall, this is really clean, and visually impressive. Good work!
| @@ -0,0 +1,167 @@ | |||
| <!DOCTYPE html> | |||
There was a problem hiding this comment.
Be sure to run your files through a validator (like https://validator.w3.org/) to check for issues in the markup. Even if a browser appears to render it properly, there may be issues that other browsers may not be so forgiving about.
| <section id="section1" class="parallax bg1"> | ||
| <!-- <h1 class="header--name"> Risha </h1> --> | ||
| <section class="aboutintro"> | ||
| <h1>Hi, I'm Risha</h1> |
There was a problem hiding this comment.
The validator warns about using h1 as a non-top-level heading. Due to the flowing way your site is set up, this might be unavoidable, but stylistically, you might consider having a top-level h1 that you hide with styling so that there is a top-level heading, but then use lower level h tags for the various sections.
|
|
||
| <h3 class="github-title"> CHECK THESE OUT!</h3> | ||
| <div class="github--items"> | ||
| <article class="github one"> |
There was a problem hiding this comment.
Try not to get so caught up on semantic tags that you end up using them in non-semantic ways. Articles should really have a good chunk of text in them and have a header of some kind (the validator is looking for anything from an h2 to an h6). If you are primarily grouping things together solely for the purposes of styling, then a good ol' div is still fine!
| <article class="bold"> | ||
| <p>Task List</p> | ||
| </article> |
There was a problem hiding this comment.
This feels like putting the project name in some sort of heading tag would be more appropriate. The project name on its own isn't really suitable to be an article, and the name is more important than a general paragraph.
| </section> | ||
|
|
||
| <!-- Portfolio section --> | ||
| <section id="section2" class="static"> |
There was a problem hiding this comment.
section tags aren't simply a new version of div. Think of them as being a significant chunk of text that would generally be labelled with a heading of some kind that may the appear in a table of contents. The section should really have a heading tag of some kind within.
If you are grouping elements primarily to assist with styling, then div is a perfectly valid tag to use.
(Even if we don't build a table of contents ourselves, screen readers will try to find articles and sections to provide visitors a way to jump around the page contents rapidly, and in some logical ordering.)
| } | ||
|
|
||
| .github .bold p { | ||
| font-weight: 900s; |
There was a problem hiding this comment.
| font-weight: 900s; | |
| font-weight: 900; |
| } | ||
|
|
||
| .bg1::after { | ||
| background-image: url('/Users/rishaallen/Developer/ada-projects/personal-portfolio-site/resources/images/charles-deluvio-iylxQ1e6sOA-unsplash.jpg'); |
There was a problem hiding this comment.
Make sure to use paths that are either relative to the style sheet or absolute to the site root rather than on your local file system. You can use a VSCode extension like Live Server to view static sites similar to how they would be served up from a web server, which can help make specifying paths easier.
| right: 0; | ||
| bottom: 0; | ||
| left: 0; | ||
| transform: translateZ(-1px) scale(1.5); |
There was a problem hiding this comment.
The scale here leaves a little white at the edge for me for the cactus. You could move this transform into each image to be able to set an appropriate per-image scale. This would also let you modify the background position per-image for nicer layout.
For instance, with the cactus, if you wanted to pull up the image a pit to see some of the pot, then instead of the transform here, you could add the following to the .b1::after rule:
background-position-y: -100px;
transform: translateZ(-1px) scale(1.6);| display: flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
| color: white; |
There was a problem hiding this comment.
I find the white text a bit difficult to read on the yellow non-image backgrounds. It's OK on one of my screens, but the other is really washed out. Try to keep contrast in mind, along with the idea that your visitors might not have properly calibrated display.
| text-decoration: none; | ||
| } | ||
|
|
||
| .footer--social > ul > li > a:visited { |
There was a problem hiding this comment.
Was this intended to not have the :visited pseudoclass on here? Right now, it duplicates the following rule, and since there's no non-visited link rule, the footer icons appear as a default blue. Removing the pseudoclass has the icons appear white whether or not I've visited the link.
Revert "removes feedback"
No description provided.