added a modal allowing the player to choose the level and a start gam… by nematanya · Pull Request #21 · ContriHUB/memory-game

nematanya · 2022-10-03T10:31:52Z

Added a start button and as the user clicks it, modal pop-ups which allows the player to choose the level of the game. Made changes according to previous UI only,

…e button for starting the game

dev-lovedeep · 2022-10-03T14:43:00Z

mention the issue number

dev-lovedeep

center the position of modal, add game rules in it. Also note that the timer should not start before the game actually start

dev-lovedeep · 2022-10-03T14:54:39Z

style.css

+  padding: 0.5em 1em;
+}
+
+.mn-lt-1 {


where is this class used

style.css

dev-lovedeep · 2022-10-03T14:55:58Z

style.css

+  font-size: 2rem;
+}
+
+.p:not(:only-child) {


try to use better names for classes. And if the element is not much significant use some selection operation to target that element instead of assigning a class

dev-lovedeep · 2022-10-03T14:56:08Z

style.css

+.modal.active {
+  display: grid;
+}
+#bttn{


use better names

use better names

Made changes !

dev-lovedeep · 2022-10-03T14:56:45Z

style.css

+*::before,
+*::after {
+  margin: 0;
+  padding: 0;
+  box-sizing: border-box;
+}


move this part to the top , after the *{ }

move this part to the top , after the *{ }

OK sure !

nematanya · 2022-10-03T16:43:06Z

mention the issue number

Issue no.#7

nematanya · 2022-10-03T16:50:13Z

center the position of modal, add game rules in it. Also note that the timer should not start before the game actually start

Trying to change it !

nematanya · 2022-10-03T17:24:57Z

center the position of modal, add game rules in it. Also note that the timer should not start before the game actually start

fixed this issue

dev-lovedeep

there are some logical mistakes:

if I do not select the level and close the modal by clicking outside it , it is also starting the game.
once the game is started the start button is still available and I am able to click on it , and that is causing a glitch in timer

suggestion:

use the index.js from previous commit, because you have changed quite a lot of thing
create a function handleLevel(level) and call this function on button click passing the level as parameter.
make the anonymous function a normal function and then call that function and pass the level to it so that others can use that level inside that function

dev-lovedeep · 2022-10-03T18:59:05Z

index.html

-            <div id="bttn"><button class="modal__close-button" onclick="closeModal()">Easy <i class="fa fa-xmark"></i></button><br></div>
-            <div id="bttn"><button class="modal__close-button" onclick="closeModal()">Medium<i class="fa fa-xmark"></i></button><br></div>
-            <div id="bttn"><button class="modal__close-button" onclick="closeModal()">Hard <i class="fa fa-xmark"></i></button><br></div>
+            <div id="align_button"><button class="modal__close-button" onclick="closeModal()">Easy <i class="fa fa-xmark"></i></button><br></div>


again, bad naming of classes

Made all the changes you suggested.pl review.

dev-lovedeep · 2022-10-03T19:00:37Z

index.html

-            <div id="bttn"><button class="modal__close-button" onclick="closeModal()">Medium<i class="fa fa-xmark"></i></button><br></div>
-            <div id="bttn"><button class="modal__close-button" onclick="closeModal()">Hard <i class="fa fa-xmark"></i></button><br></div>
+            <div id="align_button"><button class="modal__close-button" onclick="closeModal()">Easy <i class="fa fa-xmark"></i></button><br></div>
+            <div id="align_button"><button class="modal__close-button" onclick="closeModal()">Medium<i class="fa fa-xmark"></i></button><br></div>


also the size of the button denoting the easy medium hard is not equal in size

dev-lovedeep · 2022-10-03T19:02:44Z

index.js

+    console.log()
+    timer.innerHTML = "<b>" + counter + "</b>";
+}, 1000);
+setInterval();


why is this here unused

dev-lovedeep · 2022-10-03T19:04:23Z

style.css

-  box-sizing: border-box;
-}
-
 :root {


color variables are set on a project level(by the maintainer). Remove these, you don't need it for this issue

… removed to avoid glitch

dev-lovedeep

I know it might be frustrating to get a review on the name of variables or class, but it's important.

you are doing good. stay motivated 👍

dev-lovedeep · 2022-10-04T07:00:50Z

index.html

    </h3>
        <div class="shadow" id="shadow"></div>
-       <center><button class="primary-button" onclick="openModal()" >Start Game</button></center> 
+       <center><button class="primary-button" id="hide" onclick="openModal()" >Start Game</button></center> 


hide is not a better name, try something like start-btn

Changed the name from string to level

dev-lovedeep · 2022-10-04T07:03:30Z

index.js

-  modal.classList.remove("active");
-  shadow.classList.remove("active");
-  let counter = 0;
+function handleLevel(string)


use better parameter name, like level instead of string 🤣

dev-lovedeep · 2022-10-04T07:08:10Z

index.js

+timerStart();
+
+}
+function timerStart(){


as I suggested in my previous comment, do not make this a separate function, there is this anonymous function just change it to a named function, and then instead of calling the timerStart(), call that function passing the level to it. The timer logic was already there in that function

It's not working when I'm doing this idk why :(

Made changes !

dev-lovedeep · 2022-10-05T06:53:18Z

one silly bug is, I can play the game without pressing the start game btn.

It's not working when I'm doing this IDK why :(

I modified you code(at commit ab28af1) a little, and it was working correctly (you are very close)

do the following:

change anonymous function (()=>{....}) to something like const game = (level)=>{...}
put the timer code back where it was inside the game function
only keep handleLevel(level) ,closeModal(),hideButton(),openModal() outside

nematanya · 2022-10-05T16:55:26Z

const game = (level)=>{...}

Done! thank you so much for helping!

dev-lovedeep · 2022-10-05T19:18:17Z

your code is working now, but it took quite some time, and this issue was assigned to someone else after you, so won't be able to merge this PR but will give you a +5 bonus point for your work and will give accepted-tag later

nematanya · 2022-10-07T07:21:17Z

your code is working now, but it took quite some time, and this issue was assigned to someone else after you, so won't be able to merge this PR but will give you a +5 bonus point for your work and will give accepted-tag later

Any updates?

dev-lovedeep · 2022-10-07T09:53:04Z

already gave you +5 bonus

your code is working now, but it took quite some time, and this issue was assigned to someone else after you, so won't be able to merge this PR but will give you a +5 bonus point for your work and will give accepted-tag later

Any updates?

dev-lovedeep

work's fine

added a modal allowing the player to choose the level and a start gam…

ec148c7

…e button for starting the game

dev-lovedeep reviewed Oct 3, 2022

View reviewed changes

made changes to css files

fea1808

timer starts when we choose the level

7cf40a1

dev-lovedeep reviewed Oct 3, 2022

View reviewed changes

timer will only start when we choose a level and start button will be…

ab28af1

… removed to avoid glitch

dev-lovedeep reviewed Oct 4, 2022

View reviewed changes

nematanya added 2 commits October 4, 2022 12:43

changed the id name to start-btn

834eb60

Changed the anonymous function

8d58653

changed anpnymous function to game

baf485c

dev-lovedeep closed this Oct 5, 2022

dev-lovedeep added the hacktoberfest-accepted label Oct 7, 2022

dev-lovedeep reopened this Oct 31, 2022

dev-lovedeep approved these changes Oct 31, 2022

View reviewed changes

Conversation

nematanya commented Oct 3, 2022

Uh oh!

dev-lovedeep commented Oct 3, 2022

Uh oh!

dev-lovedeep left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nematanya commented Oct 3, 2022

Uh oh!

nematanya commented Oct 3, 2022

Uh oh!

nematanya commented Oct 3, 2022

Uh oh!

dev-lovedeep left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dev-lovedeep left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dev-lovedeep commented Oct 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nematanya commented Oct 5, 2022

Uh oh!

dev-lovedeep commented Oct 5, 2022

Uh oh!

nematanya commented Oct 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dev-lovedeep commented Oct 7, 2022

Uh oh!

dev-lovedeep left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dev-lovedeep commented Oct 5, 2022 •

edited

Loading

nematanya commented Oct 7, 2022 •

edited

Loading