Feedback & Code Review od Mentora Zespołu (Mateusz Nowak)#101
Feedback & Code Review od Mentora Zespołu (Mateusz Nowak)#101MateuszNaKodach wants to merge 63 commits intofeedback/mentor-zespolufrom
Conversation
Bumps [node-notifier](https://github.com/mikaelbr/node-notifier) from 8.0.0 to 8.0.1. - [Release notes](https://github.com/mikaelbr/node-notifier/releases) - [Changelog](https://github.com/mikaelbr/node-notifier/blob/v8.0.1/CHANGELOG.md) - [Commits](mikaelbr/node-notifier@v8.0.0...v8.0.1) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [ini](https://github.com/isaacs/ini) from 1.3.5 to 1.3.8. - [Release notes](https://github.com/isaacs/ini/releases) - [Commits](npm/ini@v1.3.5...v1.3.8) Signed-off-by: dependabot[bot] <support@github.com>
…rmatting #4 Automatic precommit code formatting
#9 UI Komponent | Logo Star Wars
Zmiana wprowadzona, gdyż zamiast korzystania z szablonu html użyjemy renderowania komponentów w JavaScript.
…g principles" This reverts commit fcdd2fa
* Add favicon * Fix favicon links in index.html * Add favicon 192x192 and 512x512 in html
* Create render function Create renderTemplate.js file with button rendering function * Add div button in DOM Create new div for button rendering, add local history folder to gitignore * Add styles to rendered button Add style rules for button and font * Refactor render function * #13 Button render function Button render function can render all needed buttons * #13 Add second icon to static/ui Create second icon and add to static/ui, change icons styles * #13 Tests and code refactor Add tests for Btn function and refactor code * #13 Quick fix - file name * #13 Rename btn.js to Btn.js * #13 Button click test Add test that checks if button was clicked * #13 CR fixes * #13 CR fix 2 * Fix class names * Update text size * Add hover * Render Btn in App.js Co-authored-by: PiotrWR <pwrynio@gmail.com>
…nent inside another It's also possible to assign additional class by this function.
* Finish common Btn * Fix - delete 'RedBtn" @import from style.scss * Add isSpecial proprety in Button.js * Fix tests * Update "isSpecial" value from "" for "false" * Add $color-bg--special var in _vars.scss * Add 2 tests * Add Correct & InCorrect Answer
* Part of work * Finished Computer Mind * Create computerMind.js * Fix computerMind * ComputerMind answer on asked question to computer player Co-authored-by: Mateusz Nowak <mateusz.nowak@vonage.com>
* Quick fix Add if statement in render function to prevent adding undefined class to elements classList * Render function refactor + tests * fix test refactor * Delete unused import
* #18 create modal window * #18 create scss variables for components * #18 solve an error with variable * #18 center content of the modal window * #18 problem with calling the close function * #18 add show and close functions * #18 review fixes-change MW from class to function * #18 remove comments, add tests * #18 remove button from index.html * #18 update with var from develop * #18 change modal window tests * #18 update app.js file Co-authored-by: Pawel <pawel.szambelan@flightscope.com.pl>
Co-authored-by: lamparina <anna.lamperska@gmail.com>
* pull develop * #19 create modal window content component * #19 create modal window content * #19 require the input + rewite all using render * #19 problem with functions * #19 fix for functions * #19 fix errors due to window close * #19 use filter instead of for loop * #19 tests - mission impossible * #19 tests update * Add Modal Window tests * Add Modal Window tests * Modal Window - listener on form submit * #19 fix for image * #19 comment unnecessary button * #19 test fix * #19 shorten thecorrectAnswersCounter function Co-authored-by: Pawel <pawel.szambelan@flightscope.com.pl> Co-authored-by: Mateusz Nowak <mateusz.nowak@vonage.com>
* #10 UI Component - players ranking Add component witch takes an top scores array and renders its content * #10 Test render of ranking component * #10 Rename component classes * #10 Refactor * #10 Refactor code and styles * #10 Tests * #10 Tests Add component tests * #10 After CR tests fix * #10 code refactor
… ) (#80) Witamy, aktualnie istnieje pełna wygląd oraz obsługa mainContainera w obejmującym "Rules" oraz "Raning". Kod został poprawiony, jednakże nie jest on najpiękniejszy. Występują uzasadnione obawy, że nie zdążymy upiększyć kodu przed deadline. Dlatego też udostępniamy w pełni działający kod. Aktualnie pracujemy też nad dodaniem dodatkowej funkcjonalności (renderowanie i obsługa widoku Quizu), która okazała się NIEZBEDNA, a nie jest ujęta w task'u, który mieliśmy zrobić (ani w żadnym innym task'u z projektu). Ta funkcjonalność zostanie napisana już elegancko ;) Pozdrawiamy @PiotrWr & @tomdworniczak ------------------------------------------ * #23 Add test of StarWars Triller (from PiotrR, TomaszD, PawełS) * Finish Start Window Component (Piotr Rynio i T Tomasz Dworniczak) * Add more time for StartWindow animation (Piotr Rynio & Tomasz Dworniczak) * Add TODO in Wrapper ( @PiotrWr & @tomdworniczak ) * Not Full Commit * Add wrappet tests TDD ( @PiotrWr & @tomdworniczak ) * Fix wrapper tests ( @PiotrWr & @tomdworniczak ) * Add setStartView function ( @PiotrWr & @tomdworniczak ) * Part ( @PiotrWr & @tomdworniczak ) * Next Commit * Next commit ( @PiotrWr & @tomdworniczak ) * Create game mode view switching and buttons service ( @PiotrWr & @tomdworniczak ) * Create ranking retrieving function ( @PiotrWr & @tomdworniczak ) * #23 Create separate view for rendering GameOptionsView ( @PiotrWr & @tomdworniczak ) * Fix 1/2 part (Piotr Rynio i T Tomasz Dworniczak) * Fix 1/2 part (Piotr Rynio i T Tomasz Dworniczak) * Finish Game Options View (Piotr Rynio i T Tomasz Dworniczak) * Uncomented Animation * #23 next part ( @PiotrWr & @tomdworniczak ) * Fix after Code Review
Have Fun! ( @PiotrWr & @tomdworniczak )
* #21 Create Timer component and styles Create timer function and render function to it, add styles for timer * #21 Timer styles * #21 Tests * #21 Function refactor * #21 Timer function separation of concerns * refactor * #21 tests refactor * fix * #21 refactor * #21 Code refactor * #21 timer refactor * #21 Refactor * #21 Add another callbackfuntion to MainTimer, apply styles to TextTimer * #21 Fix in callback function * # 21 Add test for TextTimer and MainTimer
* pull develop * fix for loading yoda and death star images Co-authored-by: Pawel <pawel.szambelan@flightscope.com.pl>
* pull develop * Fix - add ranking sort before return * #92 review changes Co-authored-by: Pawel <pawel.szambelan@flightscope.com.pl>
* pull develop * new view for modal window nad its content Co-authored-by: Pawel <pawel.szambelan@flightscope.com.pl>
* pull develop * remove border Co-authored-by: Pawel <pawel.szambelan@flightscope.com.pl>
* pull develop * #73 first try with responsivity * #73 grid change * #73 reposnvity for the main page * #73 responsivity for main page * #73 remove comments * #73 responsivity for start window * #73 responsivity for mode ranking view * #73 responsivity for modalWindow * #73 last fixes * #73 responsivity for answer buttons * #73 remove dead code from _main.scss Co-authored-by: Pawel <pawel.szambelan@flightscope.com.pl>
* pull develop * #73 first try with responsivity * #73 grid change * #73 reposnvity for the main page * #73 responsivity for main page * #73 remove comments * #73 responsivity for start window * #73 responsivity for mode ranking view * #73 responsivity for modalWindow * #74 start * #74 responsivity from 601 to 1024 * #74 responsivity for answer buttons Co-authored-by: Pawel <pawel.szambelan@flightscope.com.pl>
* pull develop * mobile responsivity fix Co-authored-by: Pawel <pawel.szambelan@flightscope.com.pl>
* pull develop * time fixes * fix for tests Co-authored-by: Pawel <pawel.szambelan@flightscope.com.pl>
MateuszNaKodach
left a comment
There was a problem hiding this comment.
Dodałem komentarze w w tym Pull Requescie jako składowa oceny Kod dostarczonej implementacji (0-10). Bardzo dobra robota! Super, że udało nam się w miarę nawet testować zadania. Tam opisywałem wszystko, na co normalnie zwróciłbym uwagę. W trakcie trwania projektu brałem poprawkę na to, że jest to nasz pierwszy projekt zespołowy :)
Ocena 9/10
| { | ||
| "singleQuote": true, | ||
| "trailingComma": "all" | ||
| "trailingComma": "all", |
There was a problem hiding this comment.
Super, że był skonfigurowany prettier! Dużo zespołów to pominęło, a to znacznie pomogło we współpracy.
| "homepage": "https://github.com/CodersCamp2020/CodersCamp2020.Project.JavaScript.StarWarsQuiz#readme", | ||
| "dependencies": { | ||
| "parcel": "^1.12.4", | ||
| "@types/jest": "^26.0.19", |
There was a problem hiding this comment.
Nie mieliśmy TSa więc nic nie mówiłem, ale racja, że jak już to powinno to być w devDependencies.
Nie wiem czemu VSCode nie wykrywał typów dla jesta bez tej zależności.
| @@ -0,0 +1,24 @@ | |||
| body { | |||
| width: 100%; | |||
| // color: $color-font--primary; | |||
There was a problem hiding this comment.
Wszystkie zakomentowane wartości powinny wylecieć. Po to mamy Gita, jak coś można się cofnąć, a tak to przy zmianach kod zakomentowany często może sie nawet nie kompilować (np. zmienilibyśmy nazwę jakiejś zmiennej, która była zakomentowana).
| @@ -0,0 +1,56 @@ | |||
| // * | |||
| // * nie dotykać - dobrze jest :) | |||
There was a problem hiding this comment.
Co prawda można coś takiego opisać, ale trochę w innych słowach i po angielsku, jeśli miałoby przetrwać dłuższy czas :) Pamiętajcie, że ten kod widać publicznie.
| @@ -0,0 +1,62 @@ | |||
| //Style buttonów zmieniłem na razie pod to co wskazuje projekt, pewnie będzie trzeba zrobić refactor w przyszłości - Tomek | |||
There was a problem hiding this comment.
Tak jak wszystkie inne komentarze - można usunąć.
| @@ -0,0 +1,7 @@ | |||
| export const GameMode = (text = 'Who is this character?') => { | |||
There was a problem hiding this comment.
Defaultowy argument moze byc tutaj troche mylacy.
| createHeaderRow(rankingContainer); | ||
|
|
||
| const rankingIsFilled = topScores && topScores.length > 0; | ||
| rankingIsFilled |
There was a problem hiding this comment.
Ładne wydzielenie metod, ALE
Tutaj za dużo polegania na mutowaniu obiektów. Im mniej utrzymywanie stanów / zmiennych niż stałych - tym lepiej dla czytelności i utrzymywalności kodu.
const rankingIsFilled = topScores && topScores.length > 0;
const ranking = rankingIsFilled ? RankingWithScores(topScores)
: EmptyRanking(); //Empty to empty - chyba nie wazne sa tutaj argumenty
modeRankingObj.appendChild(ranking);
Te przedrostki create tez zdaja sie nic nie wnosic do zrozumienia.
| ], | ||
| ) => { | ||
|
|
||
| // * set nav |
There was a problem hiding this comment.
Lepiej po prostu zrobić takie metody niż komentarze. Komentarz nie zawsze odzwierciedla rzeczywistość :)
Tak jak na dole return nie ustawia first itema:
// * set first item active
return navMenuDomObj;
|
|
||
| // TODO: funkcja wywołująca ustawine początkowe | ||
| // TODO: -- Mode Title | ||
| // TODO: -- Container (Rules lub ranking) |
There was a problem hiding this comment.
To już chyba wszystko zrobione.
| }); | ||
| testButton.setSuccess(); | ||
| testButton.setResetModifier(); | ||
| expect(testButton.classList.contains('button--success')).toBe(false); |
There was a problem hiding this comment.
Jeszcze popracujemy na testami :) Bo powinnismy testowac czy wlasciwosci tego sa takie jak oczekujemy, a nie czy ma przypisana klase.
No description provided.