theman550 / DAT257-K9

Project for course DAT257
1 stars 1 forks source link

Notification system #30

Closed ITJohan closed 4 years ago

ITJohan commented 4 years ago

Har skapat ett simpelt notifikationssystem som fungerar genom att man skickar funktionen showNotification till sin komponent och använder den genom att skriva showNotification(meddelande, färg för notifikationen (t.ex 'green'), hur många sekunder den ska visas). Notifikationen visas då under navbaren och försvinner efter angiven tid.

ITJohan commented 4 years ago

Går det att skriva 'showNotification' direkt i en child under App utan imports? Vet inte själv hur det fungerar i React.

Japp, man skriver t.ex. <Login showNotification={showNotification} /> för att passa funktionen till Login-komponenten. I Login-komponenten får man in den som en property (props), const Login = (props) => props.showNotification(...), eller bara const Login = ({showNotification}) => showNotification(...).

Hade varit nice med tester som går igenom hela notifikationsprocessen. Att man ifrån test-filen kallar 'showNotification' och bevisar att notifikationen dyker upp. Och sedan ett annat test fall där man visar att notifikationen försvinner efter (seconds * 1000) ms. Vet inte om det är overkill, ibland är det onödigt att testa absolut allt.

Ja det sant, är ju egentligen den funktionaliteten som är viktig. :relaxed: Ska se om jag kan mocka en driver för den.

I App.js:10 har du en // eslint-disable-next-line, är det för att showNotification aldrig används eller exporteras? Bör kunna lösas genom att ändra config eller importera React i eslint config.

Den finns där för att eslint klagar på oanvända funktioner, men som jag skrev ovanför så kan den tas bort av den som använder funktionen. Jag tycker vi ska ha kvar den eslint-konfigen då det är lätt att glömma bort oanvända funktioner.

ITJohan commented 4 years ago

Efter lite närmare eftertanke tror jag det är bättre att de komponenter som använder sig av showNotification testar den funktionen. React testing library har som filosofi att endast testa funktionalitet och inte implementation. Möjligt att App.test.js skulle kunna testa funktionen men då måste man exportera den och grejer, blir lite onaturligt.