Funda eindresultaat #22
Conversation
| <div class="slider-wrapper"> | ||
| <button id="prev-slide" class="slide-button material-symbol-rounded">←</button> | ||
| <div id="oude"class="image-list"> | ||
| <img class="modern-home" src="plaatjess/foto-s-van-een-modern-huizen-door-bas-suurmond-fotografie.webp" alt="img-1" class="image-item"> |
There was a problem hiding this comment.
denk aan je alt-text. Dit moet een beschrijving zijn waar een gebruiker wat aan heeft!
| <section class="startscherm"> | ||
| <div class="container"> | ||
| <div class="slider-wrapper"> | ||
| <button id="prev-slide" class="slide-button material-symbol-rounded">←</button> |
There was a problem hiding this comment.
Zorg dat je een button met alleen een icoon ook een accessible beschrijving geeft:
https://www.sarasoueidan.com/blog/accessible-icon-buttons/
There was a problem hiding this comment.
Je hebt veel geleerd over het bouwen van een carousel. Heel goed! Toch had je beter kunnen kiezen voor een aanpak waarbij je je eerst focust op de HTML en CSS, omdat JavaScript geen harde eis was, maar een bonus. Deze aanpak heeft er voor gezorgd dat je niet alle elementen hebt kunnen toevoegen. Je had er bijvoorbeeld kunnen voor kiezen om eerst een "placeholder" image te kunnen toevoegen en deze pas om te zetten naar een carousel, wanneer bleek dat je meer tijd over had.
Verder vind ik het goed dat je gebruik maakt van AI tools om je te helpen bij het bouwen van een carousel. Je zou je niet moeten beperken door hier fel op tegen te zijn. Zolang je deze gebruikt om niet alles voor je te bouwen, maar ook daadwerkelijk probeert om de code te begrijpen. Dit laatste kan ik niet beoordelen, maar het is meer een tip.
Heel veel succes met de eindpresentatie op school! 🚀
| @@ -0,0 +1,342 @@ | |||
| :root{ | |||
There was a problem hiding this comment.
Het zou goed zijn om de andere oude CSS bestanden die je niet gebruikt te verwijderen, zodat de pull request schoon blijft en het voor de "code reviewer" eenvoudiger is om de code te begrijpen.
| <head> | ||
| <meta charset="UTF-8"> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||
| <title>Document</title> |
There was a problem hiding this comment.
Het zou netjes zijn voor de oplevering om hier een goede titel aan te geven aangezien dit direct te zien is in de adresbalk voor de gebruiker.
| <svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" fill="currentColor" role="presentation" viewBox="0 0 48 48" class="flex mr-1 -mt-1 md:inline"><path d="M34.718 23.938l3 3.332v15.326a1.5 1.5 0 01-1.497 1.5H11.218a1.5 1.5 0 01-1.5-1.5V27.271l3-3.332v17.157h22V23.938zM23.735 4.998c.082.002.163.02.244.035.1.018.2.036.296.074.099.04.19.103.28.166.069.047.14.09.2.15.026.023.06.033.084.06L42.05 24.6a1.5 1.5 0 01-2.229 2.008l-2.106-2.339-3-3.332-11-12.216-11 12.217-3 3.332-2.101 2.333a1.5 1.5 0 01-2.23-2.007L22.598 5.48c.028-.032.068-.045.098-.073.039-.036.083-.06.125-.093.116-.089.237-.167.368-.217.06-.023.122-.032.184-.046.12-.029.24-.052.362-.052z"></path> | ||
| </svg> Mijn Huis</a> | ||
|
|
||
| <a class="alog" href="https://login.funda.nl/account/login?ReturnUrl=%2Fconnect%2Fauthorize%2Fcallback%3Fclient_id%3Dfunda.website%26redirect_uri%3Dhttps%253A%252F%252Fwww.funda.nl%252Fauth%252Fsignin-oidc%252F%26response_type%3Dcode%2520id_token%26scope%3Dopenid%2520profile%2520funda_basic%2520funda_login_metadata%26state%3DOpenIdConnect.AuthenticationProperties%253DZDC4vpi4rrahZN3bF6TK9z8O-fQ2tpjvCVStt_15IJk4_HHI9RDJkfnh3oBrW8O0JnH4VGQL0mclHpsePDTIQ9PI0dc2Iuzc8Wz9cmHslVDqqdvMyOf-3nmQnKSNoKmtTYG5enVoU8BkqTDSQJDEMw23Ut9x3i3TlgbW-NZl18HhUgBRRvwZYBtAKZOHQwxN6i4EuRWLOE2XSeng41SWDGBgeZrYUjF0MdOI7tkmtyJWHJpsV73CCQcoLIoTVTAnJKY-Ukv1sjkIkpOBw9n5LrRqfmQFkWXAYY-8Mt4q7pdK9h89%26response_mode%3Dform_post%26nonce%3D638417252361122283.ZGNjM2RlOTQtMDBjYy00NTE5LWEzMGItMDY0ZTQzYmQ1Y2MwY2EyMDI4NzEtNjMyNi00ZWY0LTkwNzQtYjYwOWVjOTFkMDEw%26x-client-SKU%3DID_NET461%26x-client-ver%3D6.7.1.0"> |
There was a problem hiding this comment.
Je hoeft de gebruiker alleen te sturen naar de login pagina, omdat je geen ReturnUrl nodig hebt in dit geval. Dit maakt het bestand beter leesbaar. https://login.funda.nl is dus voldoende.
| <!-- SLIDER SECTION --> | ||
|
|
||
| <body> | ||
| <section class="startscherm"> |
There was a problem hiding this comment.
Ik zou gebruik maken van een consistentie naamgeving voor de classes. Dus niet het mixen van stijlen en meerdere talen. Je zou eens kunnen kijken naar: https://en.bem.info/methodology/naming-convention/
|
|
||
| Bijvoorbeeld: uitvoeren van code/design reviews, user tests met gebruikers, toegankelijkheid testen met lighthouse en handmatige tests, je bevindingen documenteren en bepalen of je nog een iteratie maakt. | ||
|
|
||
| ## Definition of done |
There was a problem hiding this comment.
Mooi om te zien dat jullie gebruik maken van een Definition of Done!
| .logo{ | ||
| position: relative; | ||
| width: 100px; | ||
| height: 75px; | ||
| } |
There was a problem hiding this comment.
| .logo{ | |
| position: relative; | |
| width: 100px; | |
| height: 75px; | |
| } | |
| Volgens mij is `position: relative;` overbodig in deze situatie, maar dat kun jij beter beoordelen. | |
| .logo { | |
| width: 100px; | |
| height: 75px; | |
| } |
| display: flex; | ||
| } | ||
|
|
||
| .alog{ |
There was a problem hiding this comment.
Zorg voor een consistentie indentation in alle bestanden in het project. Je kunt dit eenvoudig oplossen door een extensie te gebruik in Visual Studio Code (als je deze gebruikt). Je kan het zo instellen dat wanneer je een bestand opslaat na het maken van een wijziging, de editor automatisch de indentation fixed. Dit zou je moeten automatiseren, zodat je er niet over na hoeft te denken.
| .alog{ | |
| .alog { |
| .slider-wrapper .slide-button#prev-slide { | ||
| left: -1px; | ||
| } | ||
|
|
||
| .slider-wrapper .slide-button#next-slide { | ||
| right: -1px; | ||
| } |
There was a problem hiding this comment.
| .slider-wrapper .slide-button#prev-slide { | |
| left: -1px; | |
| } | |
| .slider-wrapper .slide-button#next-slide { | |
| right: -1px; | |
| } | |
| .slider-wrapper .slide-button#prev-slide { | |
| left: -1px; | |
| } | |
| Bij negatieve waardes van slechts 1 pixel krijg ik altijd de kriebels. Is dit daadwerkelijk nodig? Kijk of je het niet anders kan oplossen, zodat je deze regels kunt verwijderen. | |
| .slider-wrapper .slide-button#next-slide { | |
| right: -1px; | |
| } |
| .slide-button#prev-slide { | ||
| max-width: 1200px; | ||
| } | ||
|
|
||
| .slide-button#next-slide { | ||
| max-width: 1200px; | ||
| } |
There was a problem hiding this comment.
Dit zou je ook op een eenvoudigere manier kunnen schrijven door het te hergebruiken.
| .slide-button#prev-slide { | |
| max-width: 1200px; | |
| } | |
| .slide-button#next-slide { | |
| max-width: 1200px; | |
| } | |
| .slide-button { | |
| max-width: 1200px; | |
| } |
| .slider-wrapper .image-list { | ||
| display: grid; | ||
| gap: 18px; | ||
| font-size: 0; |
There was a problem hiding this comment.
Een font-size met een waarde van 0 zou nooit nodig moeten zijn, omdat je dan blijkbaar geen tekst nodig hebt?
Voor accessibility zou je een element altijd slechts visueel kunnen verbergen met visibility, maar behouden in het Document Object Model (DOM).
|
Top bedankt voor alle feedback, ga er nog zeker veel van leren |
Ik heb gewoon miblie first vewrsie gemaakt van funda koop scherm