Skip to content

add modal(Popup) at the beginning of game #7#23

Open
ManishSheela wants to merge 2 commits intoContriHUB:mainfrom
ManishSheela:mani
Open

add modal(Popup) at the beginning of game #7#23
ManishSheela wants to merge 2 commits intoContriHUB:mainfrom
ManishSheela:mani

Conversation

@ManishSheela
Copy link

Fixed #7

  1. added name in the contributors.MD file
  2. Enhanced UI of (Start, restart, replay) buttons
  3. added popup Model at the beginning of the game
  4. wrapped some elements in a box

Copy link
Contributor

@dev-lovedeep dev-lovedeep left a comment

Choose a reason for hiding this comment

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

go through my comment on this same issue on someone else PR. link

index.html Outdated
Comment on lines 33 to 38
<button onclick="setTimer(),closeModel()">Easy</button>
<button onclick="setTimer(),closeModel()">Medium</button>
<button onclick="setTimer(),closeModel()">Hard</button>
</div>
<!-- Levels section end -->
<i class="fas fa-times" onclick="closeModel()"></i>
Copy link
Contributor

Choose a reason for hiding this comment

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

no need to call multiple functions from the button on click. Just call a generic function and pass level as its parameter

index.js Outdated
let counter = 0;
const timer = document.querySelector("#timer");
//increasing the counter
function inter() {
Copy link
Contributor

Choose a reason for hiding this comment

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

bad name inter

index.js Outdated
const box = document.getElementById("model");
const prevent_box = document.getElementById("prevent-box");
prevent_box.style.display = "block";
box.style.transition = "all 0.5s";
Copy link
Contributor

Choose a reason for hiding this comment

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

this property must be set in css file

index.js Outdated
Comment on lines 24 to 25
const box = document.getElementById("model");
const prevent_box = document.getElementById("prevent-box");
Copy link
Contributor

Choose a reason for hiding this comment

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

these you you are using in close modal also , so better to declare them on the top one time

@dev-lovedeep
Copy link
Contributor

@ManishSheela are you working on this issue?

@dev-lovedeep
Copy link
Contributor

@ManishSheela you want to continue with PR, otherwise I can assign this issue to someone else

@ManishSheela
Copy link
Author

@dev-lovedeep I'm not working on this issue. Please assign this to someone else.

@dev-lovedeep
Copy link
Contributor

you are very close to completing the issue..but if you really want I can close it

@dev-lovedeep
Copy link
Contributor

you are very close to completing the issue..but if you really want I can close it. Let me know

@ManishSheela
Copy link
Author

@dev-lovedeep I think I resolved the issue. Updating in some time

@ManishSheela
Copy link
Author

@dev-lovedeep made changes !
Please review and Let me know.
Thank you for giving me enough time

index.js Outdated
Comment on lines 23 to 25
prevent_box.onclick = ()=>{
alert(`Oops...! \nChoose your Game level by clicking the Start Game button !`);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of giving an alert, you can just open the modal to choose the level

index.js Outdated
Comment on lines 34 to 43
function closeModel(level) {
if (level != 0) {
setTimer();
prevent_box.classList.add("prevent-condition-none");
}

else {
prevent_box.classList.remove("prevent-condition-none");
}
box.classList.remove("box-condition");
Copy link
Contributor

Choose a reason for hiding this comment

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

I have merged a PR that implements the game level. It accepts levels using the prompt. Just connect the buttons you have created with that code and remove merge conflicts

@dev-lovedeep
Copy link
Contributor

reopening this issue

@ManishSheela
Copy link
Author

i fixed the issue

@dev-lovedeep
Copy link
Contributor

i fixed the issue

@ManishSheela ok reassigning back to you, but it has already been 6 days, complete it quickly.

@ManishSheela
Copy link
Author

Issue is resolved now, you may check.

Copy link
Contributor

@dev-lovedeep dev-lovedeep left a comment

Choose a reason for hiding this comment

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

remove the merge conflicts and then i will merge this PR

Comment on lines 68 to 74
document.getElementById("start-btn").addEventListener("click", function () {
removeChild();
counter = 0;
clearInterval(interval);
timer.innerHTML = "<b>" + 0 + "</b>";
popup_box.classList.add("popupBox");
});
Copy link
Contributor

Choose a reason for hiding this comment

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

the game screen is completely empty at starting, so put the start game button at the center of the screen

then you can delete this remove child function

Copy link
Author

Choose a reason for hiding this comment

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

Here removeChild function is for deleting previously stored cards because every time user chooses a new level, the cards merge with previous cards

@ManishSheela ManishSheela force-pushed the mani branch 2 times, most recently from 71b5737 to d0224af Compare October 11, 2022 08:38
@ManishSheela
Copy link
Author

I am unable to resolve the conflicts since the change in the latest commit and my code is very different and complicated.

Copy link
Contributor

@dev-lovedeep dev-lovedeep left a comment

Choose a reason for hiding this comment

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

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

add modal(Popup) at the beginning of game

2 participants