Code audit - Githubissues

Bookie0 commented 6 days ago

Hi team, here are a few issues i've found while poking through the code that we can fix to improve accessibility and maintainability of our code:

1. Proper tags

[ ] We should use more semantic HTML and use proper tags like<section> or <article> instead of so many<div>s.
- For example, in the hero section of the home page, the "Courses" section, "Outreach" section, and "Team" section on this page could each be a <section> tag.
- [ ] Or the cards in the "About" page could be <article> tags. (here)
- [ ] The website footer should be <footer>
- [ ] The navbar should be <nav> (and the individual links should be <li> list items within a <ul>)
[ ] I've noticed sometimes we use <img> (eg: here) and sometimes we use <Image> (eg: here).
- We should probably standardize this
[x] I believe for FAQ accordions here, we should use the <details> and <summary> tags. See this MDN article for more (https://github.com/cornell-dti/idol/pull/746)

2. Headers

[x] Each page should have a <h1> tag https://github.com/cornell-dti/idol/pull/755
- Eg: in page.tsx over here, our highest header is only <h2>
- And for example here, here, and then here, we're using <h1> several times. Ideally, only the hero section at the top of the page should include a <h1>
[x] We shouldn't skip over headers https://github.com/cornell-dti/idol/pull/755
- In bottom.tsx over here, we use a <h5> and then a <h6> pretty randomly, skipping over a bunch of other headers
- Same here; we use a <h3> even if the page is missing a <h2>

3. Alt text

[x] We don't need alt text for images that are decorative (but do still keep an explicit alt="" attribute) or that already have a label. https://github.com/cornell-dti/idol/pull/754
- Eg here, we already have the "LEARN MORE" label in the button, so the alt text on the <img> is redundant
[x] Many other instances of lacking alt text, like here or here in bottom.tsx. And in many other instances throughout the website too https://github.com/cornell-dti/idol/pull/754
[x] Sometimes the issue isn't missing alt text, but unprecise alt text. In general, slightly more descriptive alt text would be nice. It's good practice to specify what this image is supposed to represent (which you do well here with "Trends icon"), but could be nice as well to also describe what you see visually (in this case something like "laptop") https://github.com/cornell-dti/idol/pull/754
- Eg: here or here
[x] https://github.com/cornell-dti/idol/pull/754
- Like here or here, the image has a different alt text than its onscreen label

4. Random values

Seems like some values are sorta random.
[ ] We should try and use the standard Tailwind values
- Eg: here
[ ] For colors, we should be using CSS variables to ensure consistency throughout our website
- Eg: here or here

5. Making values constants

[ ] We could perhaps make some values constants so that we our values are more consistent. And that way, if we have to update them, we only change it once.
- Eg: here or here. In that 2nd link's case, if we're using slightly different values for similar icons, we should consider going back to the source image and editing that to make the dimensions the same.

6. Button/link labels

[x] We should avoid generic "Learn more" links as it's not accessible and is unclear https://github.com/cornell-dti/idol/pull/754
- Eg: here

7. Misc

[ ] I don't think we need an extra <div> here? We could just apply the hidden attribute directly on the <a> I think. Makes for cleaner, simpler code, and also means we are using less <div> soup (lol)

andrew032011 commented 3 days ago

Same here; we use a <h3> even if the page is missing a <h2>

For this one, the MemberGroup is a component on the team page, which has an <h2> in the subheader above, so the <h3> wouldn't be considered skipping headers on the entire page.

Bookie0 commented 2 days ago