Memory Pair Game #361
Memory Pair Game #361
Conversation
|
This issue has been automatically marked as stale because there were no activity during last 14 days. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. А. Чому так? Б. Що робити, якщо в піарі нема оновлень, оскільки не зрозуміло, що треба зробити? В. А якщо я все зробив(ла) і це ментор не рев'юває мої зміни?
Г. Хіба недостатньо того, що я додав(ла) коміт із змінами? Традиційна пропозиція: задай питання по вищенаписаному в студентському чаті. |
|
Up |
| @@ -0,0 +1,70 @@ | |||
| /* ========================================================================== */ | |||
There was a problem hiding this comment.
please remove all comments
There was a problem hiding this comment.
Удалил.
Коммит: ff500c8
| </div> | ||
| <div class="board__side flipper__side flipper__side--back"> | ||
| <ul class="board__grid" data-board-grid> | ||
| <!-- <li class="card flipper"> |
There was a problem hiding this comment.
Удалил.
Коммит: ff500c8
| </head> | ||
| <body> | ||
| <div class="page"> | ||
| <main class="board flipper"> |
There was a problem hiding this comment.
seems you have redundant spaces here and below
There was a problem hiding this comment.
Разделяю классы двумя пробелами, чтобы легче было различать.
Удалил.
Коммит: 77980d8
|
|
||
|
|
||
| function checkFlippedCards() { | ||
| score = score + 1; |
There was a problem hiding this comment.
you can use shorthand score ++
There was a problem hiding this comment.
Исправил
Коммит: 372cad9
|
|
||
|
|
||
| function shuffleArray(o) { | ||
| for ( |
There was a problem hiding this comment.
array method sort will look more clear here
There was a problem hiding this comment.
Переделал с помощью метода sort
Коммит: 85c9987
| function startNewGame() { | ||
| boardGrid.innerHTML = ""; | ||
| renderGrid(); | ||
|
|
There was a problem hiding this comment.
please remove spaces, because it is not easy to understand where the function starts and ends
There was a problem hiding this comment.
Мне тоже тяжело определить границы функции.
Но когда внутри функции нет отступов, тяжело отделить действия внутри.
Удалил.
Коммит: 202d2a1
| ? | ||
| </div> | ||
| <div class="card__side card__side--front flipper__side flipper__side--back"> | ||
| <img class="card__img" src="img/${option}.png" width="100" height="100" alt="${option}"/> |
There was a problem hiding this comment.
you can add width and height to css file to make code cleaner
There was a problem hiding this comment.
Насколько я знаю, HTML-атрибуты width и height полезны для изображений для того, чтобы браузер ещё до начала загрузки изображения выделял для них пространство.
Например, вот здесь я статью переводил когда-то: https://habr.com/ru/post/524918/
Я сейчас задал изображениям размер 100%, чтобы в случае изменения размера ячеек сетки, изображения занимали всё доступное пространство. А "флипперу" задал aspecti-ratio, чтобы он всегда был квадратным
Коммит: 1474bd7
| score = score + 1; | ||
|
|
||
| const isSame = flippedCards | ||
| .map((elem) => elem.querySelector(".card__img").alt) |
There was a problem hiding this comment.
elem is card here? Please try using clear naming, then your code will be easier to read
There was a problem hiding this comment.
Using alt attribute to compare cards is a bad approach, you can use alt only for describing images, that's it. So, you can compare unique identifiers id for example
There was a problem hiding this comment.
elemiscardhere?
Да
elem is card here? Please try using clear naming, then your code will be easier to read
Понял.
Исправил и постараюсь учесть на будущее.
Using
altattribute to compare cards is a bad approach, you can use alt only for describing images, that's it. So, you can compare unique identifiersidfor example
Вместо атрибута alt использовал значение src
Коммит: 76975f9
| boardGrid.addEventListener("click", forCardFront_onBoardGrid_Click_Handler); | ||
|
|
||
|
|
||
| function changeGameSide() { |
There was a problem hiding this comment.
let's move to the evil game side :) Please improve the naming a bit. Try to read your code and understand what it does in every step
There was a problem hiding this comment.
Заменил "игру" на "доску" в названии функции
Коммит: 9026d23
|
|
||
| const cardFlippers = Array.from(boardGrid.querySelectorAll(".flipper__inner")); | ||
|
|
||
| cardFlippers.forEach((cardInner) => flipCard(cardInner)); |
There was a problem hiding this comment.
:)))))))
Смешно.
Но слово "flipper" я взял из этой статьи.
Насколько я понимаю, значит что-то вроде "переворачивалка".
Не знаю, чем заменить.
Или заменять и не обязательно?
|
@kasionio , надеюсь, про мой PR не забыли? |
Memory Pair Game
Demo |
Code base
The code is submitted in a dedicated feature branch.
Only code files are submitted.
Please, review.