Automatically fill in the fourth hand when three hands have exactly 13 cards each#235
Automatically fill in the fourth hand when three hands have exactly 13 cards each#235tameware wants to merge 8 commits into
Conversation
When three hands each hold 13 distinct cards and the fourth hand is empty, enable a Fill fourth hand button and automatically complete the deal with the remaining 13 cards. Includes JS unit tests for the new logic. Co-authored-by: Adam Wildavsky <adam@tameware.com>
Use Enter for filling an eligible fourth hand or running double-dummy on a complete deal, while styling unavailable actions clearly. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Pull request overview
This PR enhances the DDS MVP web UI so that when three hands are complete (39 cards total), users can automatically fill the missing fourth hand, and it refines “Enter” key behavior to trigger the appropriate default action (fill vs. double-dummy).
Changes:
- Added a “Fill fourth hand” button and supporting logic to compute and populate the missing hand from the remaining 13 cards.
- Updated action availability/keyboard handling so Enter triggers fill when eligible, otherwise runs double-dummy only when all hands are complete.
- Expanded coverage with new/updated E2E, JS unit, and CSS contract tests, and added CSS styling for enabled/disabled action buttons.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| web/tests/test_mvp_e2e.py | Adds E2E coverage for Enter key behavior and the new Fill fourth hand flow; adjusts incomplete-deal validation test expectations. |
| web/tests/test_dds_mvp_css.py | New CSS contract tests asserting enabled/disabled styling for the primary action buttons. |
| web/tests/dds_mvp_test.mjs | Extends unit tests and mocks to cover fourth-hand fill state, action-button enablement, and Enter key routing. |
| web/dds_mvp.js | Implements fourth-hand fill computation, action-button state updates, and global keydown handling. |
| web/dds_mvp.html | Adds the Fill fourth hand button and IDs; makes buttons type="button"; defaults action buttons to disabled. |
| web/dds_mvp.css | Styles enabled/disabled states for the primary action buttons. |
| web/BUILD.bazel | Registers the new CSS py_test and includes it in the web test suite. |
The double-dummy action is disabled until all hands are complete, so its incomplete-hand error path and tests cannot execute through the UI. Co-authored-by: Cursor <cursoragent@cursor.com>
Show all 52 cards by suit and gray cards as they are entered so users can see which cards remain available. Co-authored-by: Cursor <cursoragent@cursor.com>
Display a live card count only when a hand exceeds 13 cards so overfilled hands are immediately identifiable. Co-authored-by: Cursor <cursoragent@cursor.com>
|
I thought of a better way to do this. Converting to draft… |
Remove the Fill fourth hand button and fill it automatically once three hands each hold 13 cards, leaving Enter for double-dummy on a full deal. Co-authored-by: Cursor <cursoragent@cursor.com>
tameware
changed the title
Limit the Enter shortcut to hand inputs, show the default outline only when Enter applies, and focus Double-dummy after loading a test deal. Co-authored-by: Cursor <cursoragent@cursor.com>
Reject incomplete deals before invoking DDS so direct calls receive a clear validation message instead of an engine error. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Overall this is getting further and further removed from the concept of minimum viable product and seems to be trying to approach a more mature user interface. That said, will comment on implementation. |
| const fullHands = handCounts.filter((count) => count === 13).length; | ||
| const emptyHands = handCounts.filter((count) => count === 0).length; | ||
| const partialHands = handCounts.filter((count) => count > 0 && count < 13).length; | ||
|
|
||
| if (fullHands !== 3 || emptyHands !== 1 || partialHands > 0) { | ||
| return { canFill: false }; | ||
| } |
There was a problem hiding this comment.
This feels too constraining. If the user enters one card in west and then populates north, south, and east, you won't autofill the rest of the west hand. But you would if west were left blank.
This could be intentional.
There was a problem hiding this comment.
It was intentional, but I prefer your suggestion! Will implement it, then resolve this comment.
| if (!PIPS.includes(pip)) { | ||
| return { canFill: false }; | ||
| } | ||
|
|
||
| if (usedCards[card]) { | ||
| return { canFill: false }; | ||
| } |
There was a problem hiding this comment.
These are confirming something that should be true by invariance. If there are duplicate or non-bridge cards assigned to a hand then something has really gone wrong in a way that isn't going to be fixed by refusing to deal out the rest of the deck.
|
Agreed, it's becoming more than an MVP. That's in part because I want to use it myself! Would you suggest moving it to its own repo, @tzimnoch ? |
|
I struggle to think of ways to make significant simplifications and clean up, and not for lack of trying the entire exercise myself. My own project is in React, which allows some modularization of components. This is not an easy exercise. |
|
I shared my private repo. Feel free to have a look around. It's not actually in any cleaner a state, but you might find implementation ideas you're welcome to steal. If I were less embarrassed by the current quality of it, I would make it public. The latest build I bothered to push is testable at https://tzimnoch.github.io/cbb.testsite/ |
zzcgumn
left a comment
zzcgumn
left a comment
There was a problem hiding this comment.
My web knowledge is very limited, I agree that the use case has real value.
Deployed at https://tameware.com/adam/bridge/dds/dds_mvp.html