MarsGotBars / the-client-website

Ontwerp en maak een website voor een opdrachtgever en bespreek het resultaat tijdens de Sprint Review
https://marsgotbars.github.io/the-client-website/
MIT License
0 stars 0 forks source link

Code Review - Sprint 2 Week 1 - Amber & Julia #13

Open julia-stevens opened 1 week ago

julia-stevens commented 1 week ago
          ![20240925_122124](https://github.com/user-attachments/assets/330472bf-e94f-40cf-bbee-5d07fabb2cec)

prototype index | Home

Originally posted by @MarsGotBars in https://github.com/MarsGotBars/the-client-website/issues/5#issuecomment-2373698997

julia-stevens commented 1 week ago

Zie validator: https://validator.w3.org/nu/?doc=https%3A%2F%2Fmarsgotbars.github.io%2Fthe-client-website%2F

De HTML validator geeft vrij veel errors aan, er zijn veel overkoepelende, hier de belangrijkste:

  1. image
  2. image

    De article elementen hebben geen header en die moet er wel in, zie bij 'usage notes': https://developer.mozilla.org/en-US/docs/Web/HTML/Element/article

  3. image

    Een aantal afbeeldingen heeft geen alt, deze moet er wel in staan: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/img

  4. image

    Bij de section elementen heb je een aantal keer geen heading, deze moet er wel bij, zie 'usage notes': https://developer.mozilla.org/en-US/docs/Web/HTML/Element/section

julia-stevens commented 1 week ago

https://github.com/MarsGotBars/the-client-website/blob/99059ba86efd33abb81e59db7978028fcf79a71c/index.html#L11 Je schrijft alle content van de body in een div class="container" - Misschien is deze div niet nodig, aangezien deze content al samen staat in de body? Misschien kun je hier nog naar kijken.

https://github.com/MarsGotBars/the-client-website/blob/99059ba86efd33abb81e59db7978028fcf79a71c/index.html#L33-L43 Je gaat hier van een h1 naar een h6 en slaat hiermee een aantal heading levels over. Dit is eigenlijk niet de bedoeling, zie usage notes: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/Heading_Elements

https://github.com/MarsGotBars/the-client-website/blob/99059ba86efd33abb81e59db7978028fcf79a71c/index.html#L106-L206 Je nest veel section elementen onder elkaar, misschien kun je dit vereenvoudigen?

ambersr commented 1 week ago

De structuur van je website ziet er goed uit! Je ziet duidelijk de structuur opbouw, bovenaan de website een navigatie en onderaan een footer. Ik zie wel dat er afbeeldingen verschijnen als de css uitstaat (zie afbeelding).

image
julia-stevens commented 1 week ago

https://github.com/MarsGotBars/the-client-website/blob/99059ba86efd33abb81e59db7978028fcf79a71c/index.html#L80 Je gebruikt hier een h3, maar hebt geen h2 toegepast in je code. Het is niet de bedoeling dat je h-levels overslaat, zie MDN: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/Heading_Elements

https://github.com/MarsGotBars/the-client-website/blob/99059ba86efd33abb81e59db7978028fcf79a71c/index.html#L109 Je img hebben allemaal geen alt, deze hoort er wel in te staan, vanwege screen readers: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/img

ambersr commented 1 week ago
  1. Alle onderdelen zijn ontworpen.
  2. Voor de leden pagina zie ik nog geen ontwerpen voor meerdere schermbreedtes. Misschien zou je voor de ledenpagina nog een ontwerp voor tablet en mobiel kunnen maken.
  3. In het ontwerp zie je alleen niet de viewport lijn. Misschien kan je deze nog toevoegen.
  4. Er is goed rekening gehouden met visuele hiërarchie. De eerste heading op de pagina is het grootst en valt het meest op ook de buttons hebben een goede opvallende kleur.