-
Notifications
You must be signed in to change notification settings - Fork 36
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
Static-Site #29
base: master
Are you sure you want to change the base?
Static-Site #29
Conversation
Static SiteWhat We're Looking For
|
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.
Web page code comments:
(I'll comment on code that made it into your final site. i.e. it looks like code-journal/entry-one.html wasn't included, so skipped that one.)
I noticed on several pages you have ids in your html tags for css purposes. Could they be placed in a different element on each page and achieve the same results?
(Putting ids on the html tag was a little confusing for me, as I generally consider the html and head tags to be more for HTML structure/setting browser info vs. CSS.)
There's something else called lang that you can add to the html tag to set the primary language of the page. It's . I didn't use it in my own static site so I don't think it's covered in the curriculum, but it's good to include for accessibility purposes.
I found it by using a browser plug-in, that I learned about at an accessibility class, on your github.io site. It's at https://www.deque.com/products/axe/ if you'd like to try it out--it can be fun to audit interesting websites to see what is/isn't present from an accessibility perspective.
I like your use of alt for images throughout! I find it helpful to think about how alt text would be read out loud by a screen reader in the flow of the document--would it be jarring?
I also ask myself whether I've gone into too much detail/not enough detail about what the image is, or if my details are meaningful to someone who potentially can't see images at all or for whom they're very blurry.
Your HTML structure/tag selections seemed good to me! Even before looking at your github.io site, I found it easy to know what part of the web pages I was looking at via your code. (I saw your instructor's review mentioned sometimes divs were used where sections might have been better. I had a hard time with that myself, having first gotten exposure to HTML without the concept of semantic tags. So not sure which your instructor was referring to, but the struggle is real. :) )
CSS code comments:
I like your use of comments to make it clear what html pages each section refers to! A great strategy when you're keeping all the css code in one file.
You could also have a main.css file with supplemental css files for individual pages' changes. This would reduce the number of ids you need for each page and help dry up that css code.
(I personally found pretty time-consuming to implement during classroom, though. I only managed it for meowspace I think. But I like it as a refactor in theory--maybe something to consider if you continue with front-end and have more time later on in your career?)
Good use of p:nth-child(n+3)::before in line 39 and section:first-of-type in line 308! I find those kinds of css selectors hard to wrap my head around sometimes; good job in including them!
You did some neat stuff that shows you put a lot of effort into this assignment, like animations and the iframe. (Neither of which I had the design chops to play around with for my site; I'm impressed!)
General site comments:
It was really easy to navigate through your site, and it looks fantastic when rendered!
One thing I noticed from an accessibility perspective:
I did find at times the text was a little too small for me to easily read at times, particularly when the background wasn't plain white (i.e. an image was there instead.) It seems to me the aesthetic was to emphasize certain sections and have great contrast between text sizes, but I wonder if that aesthetic could still be achieved with slightly larger sizes on the lower end of the spectrum (or perhaps different colors/bolding?)
There's tools online I've found helpful in trying to see if text/background contrast is great enough for accessibility purposes. I've liked https://snook.ca/technical/colour_contrast/colour.html. It's not fancy enough to check an image against text color, but it's got nice options to modify aspects of the color so you could check prominent colors in an image. There might be ones online that let you check images directly--not sure, but Google it if interested. :)
But purely from a design perspective the site looks great! Great job on the assignment!
Static Site
Congratulations! You're submitting your assignment!
Comprehension Questions