Skip to content
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

3 Eerste opzet create playlist feature Patrick #11

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Patrickkhr
Copy link
Collaborator

Ik ben begonnen met het maken van de feature "create a playlist" als je klikt op de knop playlist toevoegen komt er een overlay in beeld waarbij je je nieuwe playlist een naam en afbeelding kan geven en als je dat hebt gedaan kan je klikken op create playlist. Ik heb dit gemaakt volgens PE en om het pleasurable te maken heb ik een transition toegevoegd bij het openen en sluiten van de overlay.

Wanneer er een playlist gemaakt wordt moet de playlist met de gekozen naam en afbeelding worden toegevoegd aan het lijstje met playlists maar hier ben ik nog niet aan toe gekomen.

Verder moet er gecontroleerd worden of ik me aan onze code afspraken heb gehouden en of er geen conflicten zijn.

image image

Onze code afspraken:

Code conventies
Bij het inspringen hebben we afgesproken dat we het laten inspringen per element

Nesting css
Voor de specifiekere css maken we gebruik van nesting.

Classnames
Bij classnames maken we gebruiker van een '-' tussen worden en maken we geen gebruik van bovenkast.

https://github.com/lisavanmansom/pleasurable-ui/wiki/Week-1#code-conventies

@zenitba
Copy link
Collaborator

zenitba commented May 24, 2024

Tips voor betere Pull request

  • Test je wijzig met bijbehoren test resultaten
  • Voeg een livelink toe
  • Voeg een tekstvlak toe hoe een reviewen je wijzigen kan testen
  • Geef je pr een label zo heb je overzicht van de feedback die je ontvangt.
  • Voeg de code toe van de wijziging zodat je ook feedback ontvang op je code.

Tops

  • Mooi dat je aan de code conventies houdt
  • Goed dat je afbeeldingen van de wijziging/feature hebt toegevoegd
  • Duidelijk uitleg van de wijziging

@sannevanseeventer
Copy link
Collaborator

Ziet er goed uit!

Wat kleine tips voor het schrijven van een iets overzichtelijker pull request:

  • Schrijf ook een duidelijke titel in het comment veld.
  • Gebruik headings per categorie in je PR, dit zou het al een stuk overzichtelijker maken.
  • Voeg volgende keer een live link toe aan je PR.
  • Je zou volgende keer ook de resultaten van de accessibility test en performance kunnen toevoegen.

Hier is een link naar een pull request, hier zou je naar kunnen kijken als voorbeeld.
fdnd-agency/wogo#11

Maarr goed bezig!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants