Skip to content

Made App Responsive and Dynamic#26

Merged
prrockzed merged 10 commits into
prrockzed:mainfrom
ThePhoenix08:responsive-2
Dec 22, 2024
Merged

Made App Responsive and Dynamic#26
prrockzed merged 10 commits into
prrockzed:mainfrom
ThePhoenix08:responsive-2

Conversation

@ThePhoenix08

Copy link
Copy Markdown
Contributor

🚀 Pull Request Overview

Closes Issue #2

Description

This PR is an extension of the #12. It includes the requested modifications and corrections asked. Following is the summary of the PR.

From #12

  • Making chessboard responsive
    • Chessboard grid size and width height is now dynamic, its calculated whenever viewport changes using useEffect
    • Chessboard boundaries are now dynamically calculated using useRef and useEffect on resize only not everytime a piece is dragged
    • Current Player is hoisted to a ChessProvider and ChessContext, which then feeds it to Player Nameplates
  • Revamped Player Nameplates
    • Added a chessWrapper parent which holds the chessboard and player panels / nameplates in it
  • Made Modal responsive
    • CSS changes in modal for responsiveness

Now the modifications and corrections done in this PR

  • Removed unnecessary interactionMode toggle, back to the spontaneous method of choosing to drag-and-drop or select-move chess piece movement method
  • Improved player nameplates css
  • Inculcated some of the latest PRs as the branch was old (created a temp-main and merged into responsive-2 and resolved the merge conflicts)
  • fixed "underfinedR.png" fetch error during initialization, now it defaults to fetching red team piece images at initialization, caused by pawn promotion function returning undefined at first component mount / render.

Summary:

  • Bug Fix 🐛
  • New Feature ✨
  • Custom 🔧

🐛 Bug Fix:

  • Issue Fixed: Issue not created

  • Description of Fix: At initial render, the modal although not visible, is mounted to DOM. It tries to fetch the four pawn promotion options images but due to no trigger the pawn promotion function returns undefined as the team. This results in "undefinedR.png" etc being fetched. To resolve this issue, i added type guards on pawn promotion functions parameters and added a fallback to team RED just in case, for example initialization where although the modal is hidden, it has mounted.

  • Steps to Verify:
    Steps to reproduce the bug (as described in the issue).

  1. Head to main original deployment "https://prrockzed.github.io/chessed/"
  2. Check the console in developer tools of the browser
  3. You will see the following
    image
  4. If head to the network tab of developer tools and refresh the application you will see the following
    image

Actions to confirm the fix works as expected.

  1. Head to this PRs development deployment: "https://thephoenix08.github.io/chessed/"
  2. Check the network tab of developer tools and refresh the application
  3. You will see now that the fallback to RED team works, and right images are being fetched.
    image

Note - (This is a temporary solution in my opinion but uses minimum changes, a more precise will be to conditionally render the complete modal using ReactJS instead of just classList toggling to prevent such problems.)


Additional Context

Screenshots of Mobile View and Desktop View of the new UI

thephoenix08 github io_chessed_(iPhone SE) thephoenix08 github io_chessed_(Nest Hub Max)

to test please check out the dev deployment of this PR - https://thephoenix08.github.io/chessed


Open-Source Programs

Yes, I am making this PR as part of KWoC student contributor. I would request the mentor to please review this Pull Request.


✅ Checklist:

  • My code adheres to the code style of this project.
  • I have run the appropriate tests for this change.
  • I have updated the documentation (if needed).
  • My PR references any relevant issues (if applicable).
  • Breaking changes?

@ThePhoenix08

ThePhoenix08 commented Dec 18, 2024

Copy link
Copy Markdown
Contributor Author

Disclaimer

Actually, these are breaking changes as the chessboard.tsx is completely revamped. I did make it compatible to some of the last PRs made by @Bhoomish-Patel and @dipamsen, including the king highlight on checked #19 and user-select none on body while chess piece dragging #17 . Although as the PR updates complete code of the chessboard.tsx the current on review PRs of both of them will be directly affected if this PR merges. As this PR changes UI, introducing a chessWrapper component and player nameplates, as well introduce a design law of "position: fixed" to the complete application that is the chessboard, player nameplates, modal, etc, it will affect the navbar footer #18 and chess piece history tracking right table #25 .

I would request you (mentor) to decide which PR shall follow the rest, as either this one will break if there's is merged first, or there's will break if this gets merged. The breaking can be resolved easily though as my code is well aligned with the prior code (it uses the same logic as before but aims to improve readability, removes prop drilling of current player and removes playerName to chessWrapper > playerPanels), it also introduces a wrapper and a context, although css changes will be the ones which will take longer to resolve.

@dipamsen

dipamsen commented Dec 18, 2024

Copy link
Copy Markdown
Contributor

It looks great! One suggestion I have is to separately style the player name of the lost player(s) (maybe more dark background or something) to indicate they are no longer a part of the game.

@ThePhoenix08

Copy link
Copy Markdown
Contributor Author

Yea, you (@dipamsen) are right, the grey color is a little misleading as it shows both idle and lost. I have implemented what you suggested in recent commits. Now the playerState is clearly defined 'active' | 'inactive' | 'lost'. If lost, the player nameplate turns dark grey and the text shows 'Red Player Lost' for further clarity.

Mobile Desktop
thephoenix08 github io_chessed_(iPhone SE) thephoenix08 github io_chessed_(Nest Hub Max)

@ThePhoenix08

Copy link
Copy Markdown
Contributor Author

Hey, @prrockzed, sorry for mentioning you, but could you please review this PR by 25th if possible.

@prrockzed

Copy link
Copy Markdown
Owner

Hi, @ThePhoenix08!
Sorry to respond so late. I was busy in some personal and college related issues.
Great work. I am merging your PR.

@prrockzed prrockzed merged commit 114a0fc into prrockzed:main Dec 22, 2024
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