search

pandeyganesha / digit-dare

https://pandeyganesha.github.io/digit-dare/
MIT License
3 stars 7 forks source link

Remove focus from restart button #7

Closed Banh983 closed 1 week ago

Banh983 commented 1 week ago

So i do a review on your code, interest game btw, so to fix your issue i set the restart button to blur, then just to make sure it wont focus on the restart button upon the next round i set the focus to somewhere else the the document body. You right it just need 2 line of code: image

pandeyganesha commented 1 week ago

@Banh983 Great ! But you seemed to have refactored the code. Please make the changes needed to solve the issue only. Most changes seem to be changing single quotes into double quotes.

Please revert back refactoring changes. Also restartButton.blur() seems to be sufficient, we don't need .focus

If you like to refactor the code and add documentation, you should create a new branch and raise separate PR. Thanks, Will merge as soon as you fix this

Banh983 commented 1 week ago

Hi, sorry for the inconvenience, it happens because of the prettier setting on my computer, it made all the double quotes become single quotes, i restore the original state, can you check for merge please

pandeyganesha commented 1 week ago

@Banh983 I can still see those changes in this PR. I actually fixed the things here https://github.com/pandeyganesha/digit-dare/commit/751c9d97fc235a16a015f2d97c47b2feb289122f and was about merge changes but you force-pushed the changes.

image

You can yourself check the changes you have made by clicking on files changed

Please fix and review yourself either by using git diff or UI version of git diff before pushing any changes. Thanks

Banh983 commented 1 week ago

Oh i'm terribly sorry i using github desktop and it my first time solving a problem too, in the push that i made i did restore all of the single quote to double quote or any other changes, i made it look like in the original state, and i remove that .focus() line as you suggested

pandeyganesha commented 1 week ago

Oh i'm terribly sorry i using github desktop and it my first time solving a problem too, in the push that i made i did restore all of the single quote to double quote or any other changes, i made it look like in the original state, and i remove that .focus() line as you suggested

No, it's fine. That's how we learn, we make mistakes, we learn from it. It's fine. Take your own time. Always review the changes before committing and pushing the changes.

Banh983 commented 1 week ago

Thank you for your understanding, I really appreciate it, you can preview the code to see if it still have any problem I'm fixing it right away

pandeyganesha commented 1 week ago

Yes the refactored code is still there, also i can't see any new commit since my last reply.

Banh983 commented 1 week ago

I'm sure that i did made those changes as you required and i did commit it right here image

Banh983 commented 1 week ago

Ohh i see the problem now, english actually not my mother tongue so i something have a different understanding in sentences,

Banh983 commented 1 week ago

I think the code look okay, can you check for merger please 🤧🤧

pandeyganesha commented 1 week ago

Now It is fine. I can see that the refactored code has been removed. Thanks

Here are some tips that might help you in future.

  1. Always give helpful commit message. Example: "Removed the refactored code and fixed the issue" is better than "Modify again"
  2. Always review changes before committing.
  3. and for making changes create new branches like feature/<description-of-the-task> or if you are fixing a bug then name it something like bugfix/remove-focus-from-button. And then make changes in that branch
  4. Sync your code from the original repo before raising PR and then raise PR from the branch.

Nevertheless Thank you for your contribution. Merging and closing the PR.