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

Develop #1039

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Develop #1039

wants to merge 8 commits into from

Conversation

vikaruda
Copy link

No description provided.

Copy link

@etojeDenys etojeDenys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. provide the demo link in the pr description
  2. remove all console.log and redundant comments

Comment on lines 80 to 106
if (e.key === 'ArrowRight') {
// eslint-disable-next-line no-console
console.log('Right');
game.moveRight();
renderField(game.state); // Відображаємо оновлене поле після руху
}

if (e.key === 'ArrowLeft') {
// eslint-disable-next-line no-console
console.log('Left');
game.moveLeft();
renderField(game.state); // Відображаємо оновлене поле після руху
}

if (e.key === 'ArrowUp') {
// eslint-disable-next-line no-console
console.log('Up');
game.moveUp();
renderField(game.state); // Відображаємо оновлене поле після руху
}

if (e.key === 'ArrowDown') {
// eslint-disable-next-line no-console
console.log('Down');
game.moveDown();
renderField(game.state); // Відображаємо оновлене поле після руху
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

try to simplify it using the switch case

@vikaruda vikaruda requested a review from etojeDenys November 26, 2024 16:15
Copy link

@etojeDenys etojeDenys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and provide the demo link

// eslint-disable-next-line no-console
console.log(initialState);
console.log(this.initialState);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove all logs

Comment on lines 152 to 156
// Об'єднуємо однакові числа, починаючи знизу
// рухаємось знизу вверх по колонці, не рядку
// якщо ми бачимо, що цей елемент {і} === {і - 1}
// тобто дорівнює попередньому елементу, то ми множимо числа на 2
// TODO: зробити типу такої логіки з двійкою, але можна винести

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and remove all comments

@vikaruda
Copy link
Author

@vikaruda vikaruda requested a review from etojeDenys November 27, 2024 07:36
Copy link

@volodymyr-soltys97 volodymyr-soltys97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job 👍
Lets' improve your code

  1. Add a favicon on the page
image
  1. Add the demo link to the PR description
image
  1. Hide this message after starting the game
image image
  1. Need to show the lose message when game is over
image

*/
constructor(initialState) {
// TODO: Замінити по індексу числа

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove all comments

Suggested change
// TODO: Замінити по індексу числа

@vikaruda
Copy link
Author

_ No description provided. _

Copy link

@etojeDenys etojeDenys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you shouldn't create a cell if the board didn't change

Screen.Recording.2024-11-28.at.11.38.28.mov
  1. looks broken
  2. remove the lose message on restart
image

@vikaruda vikaruda requested a review from etojeDenys November 30, 2024 07:03
Copy link

@etojeDenys etojeDenys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this comment still not fixed
image

@vikaruda vikaruda requested a review from etojeDenys December 2, 2024 12:37
Copy link

@volodymyr-soltys97 volodymyr-soltys97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job! 👍

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.

3 participants