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

first commit #1172

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

Conversation

SerhiiUnhurian
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.

https://SerhiiUnhurian.github.io/js_2048_game/

fix the demo link and then re-request the review

Copy link

@VitaliyBondarenko1982 VitaliyBondarenko1982 left a comment

Choose a reason for hiding this comment

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

After start game nothing happening after press on any arrow button.


/**
* Resets the game.
*/
restart() {}
restart() {
this.initGame();

Choose a reason for hiding this comment

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

after restart game board should be without any tiles.

Copy link

@maxim2310 maxim2310 left a comment

Choose a reason for hiding this comment

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

Good work, but lets make some improvement

  1. i can't move at all
  2. you need to hide this benner after click start
    image
  3. hide cells when you haven't started yet after restart
    image

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.

remove lose message on restart click
image

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.

still not fixed

@SerhiiUnhurian
Copy link
Author

I'm making new deployments but for some reason they don't appear,
I just made a new one

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 👍
To improve:

  1. Add a favicon on the page
image
  1. You need to hide the lose message after started the new game
image image

@SerhiiUnhurian
Copy link
Author

check pls

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.

Almost done!

Comment on lines 168 to 181
// updateTable() {
// const cells = document.querySelectorAll('.field-cell');
// let index = 0;

// this.board.forEach((row) => {
// row.forEach((cellValue) => {
// const cell = cells[index++];

// cell.textContent = cellValue !== 0 ? cellValue : '';
// eslint-disable-next-line max-len
// cell.className = `field-cell ${cellValue ? 'field-cell--' + cellValue : ''}`;
// });
// });
// }

Choose a reason for hiding this comment

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

Remove all comments

Copy link

@vadiimvooo vadiimvooo left a comment

Choose a reason for hiding this comment

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

Well done

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.

6 participants