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

- Fix: vertical promotion dialog bottom overflow #180

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Epitomaniac
Copy link
Contributor

  • Vertical promotion dialog no longer overflows out of the board on bottom rank promotions.
  • Small change making the order of promotion piece icons reversed on bottom rank promotions.

Copy link

vercel bot commented Jan 9, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-chessboard ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 19, 2025 5:55pm

Comment on lines +24 to +25
(boardOrientation === "white" && promoteToSquare?.[1] === "1") ||
(boardOrientation === "black" && promoteToSquare?.[1] === "8");
Copy link
Owner

Choose a reason for hiding this comment

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

what happens if the board is more than 8x8? (see #174)

I think we may need to add a new property/properties to allow users to set the promotion rank for each colour, and default it to normal values

Copy link
Contributor Author

@Epitomaniac Epitomaniac Jan 19, 2025

Choose a reason for hiding this comment

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

As long as we have access to the number of ranks, we can set its value for black and 1 for white. I don't think a new property is required unless there's a variant that allows promotion on a rank that's not the first or the last one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, the calculation for translate effect assumes the board to be 8 by 8, so this also needs to change for custom boards based on the board dimentions.
transform: translate(${-boardWidth / 16}px, ${-boardWidth / 16}px)

Comment on lines 72 to 81
{
// Reversing the order in which piece icons appear for vertical dialog if promotion occurs on the bottom rank
isBottomRank && promotionDialogVariant === "vertical"
? promotionOptions
.reverse()
.map((option) => <PromotionOption key={option} option={option} />)
: promotionOptions.map((option) => (
<PromotionOption key={option} option={option} />
))
}
Copy link
Owner

Choose a reason for hiding this comment

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

the logic for this can sit outside the JSX to make it look a bit tidier

const orderedPromotionOptions = isBottomRank && promotionDialogVariant === "vertical"
  ? promotionOptions.reverse() 
  : promotionOptions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

2 participants