Web - #248
Merged
Merged
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>
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>
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>
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>
Use one bare symbol map and apply red styling contextually so suit symbols retain their color independently of deck card pips. Co-authored-by: Cursor <cursoragent@cursor.com>
Restrict red styling to heart and diamond suit symbols so card ranks use black until entered, when they become gray. Co-authored-by: Cursor <cursoragent@cursor.com>
Use the existing direction names throughout internal hand maps, eliminating duplicate letter mappings and conversion helpers while preserving external PBN encoding. Co-authored-by: Cursor <cursoragent@cursor.com>
Centralize the S-H-D-C sequence so PBN formatting, deck rendering, and suit holding logic cannot drift apart. Co-authored-by: Cursor <cursoragent@cursor.com>
Represent suits by descriptive names throughout UI logic and convert to letter codes only at card and PBN boundaries. Co-authored-by: Cursor <cursoragent@cursor.com>
Keep named suits and pips together internally, converting to compact card keys only at DOM, map, and PBN boundaries. Co-authored-by: Cursor <cursoragent@cursor.com>
Remove redundant per-card guards while preserving duplicate and invalid-input behavior through the aggregate checks. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR delivers multiple UI/UX improvements to the DDS web MVP page, including richer deal-entry feedback and stronger automated coverage (E2E + unit + CSS contract tests) to keep the UI behavior stable.
Changes:
- Add live UI affordances while entering hands: deck-status visualization (with entered cards grayed) and per-hand card-count notes when exceeding 13 cards.
- Improve “default action”/keyboard flow by disabling/enabling and focusing the “Double-dummy it!” button appropriately (including Enter-to-run behavior).
- Expand test coverage: new CSS contract tests, expanded Node unit tests, and additional Playwright E2E scenarios; wire new test into Bazel
web_tests.
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/dds_mvp.js | Adds card modeling and new UI-update logic (deck status, card counts, default action, auto-fill fourth hand, Enter handling). |
| web/dds_mvp.html | Adds deck-status container, per-hand card-count elements, and IDs/attributes needed for JS + accessibility and tests. |
| web/dds_mvp.css | Styles for deck-status, entered-card graying, card-count notes, and default-action outline/disabled state. |
| web/tests/dds_mvp_test.mjs | Expands Node unit tests and DOM mocking to cover new behaviors and rendering. |
| web/tests/test_mvp_e2e.py | Adds Playwright E2E coverage for deck-status, card counts, focus behavior, Enter-to-run, disabled states, and 4th-hand autofill. |
| web/tests/test_dds_mvp_css.py | Adds CSS “contract tests” to assert critical selector rules remain present and consistent. |
| web/BUILD.bazel | Adds the new CSS py_test and includes it in the web_tests suite. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
tameware
marked this pull request as ready for review
July 19, 2026 13:40
Lock in that auto-fill eligibility rejects non-bridge pips and null card entries. Co-authored-by: Cursor <cursoragent@cursor.com>
tzimnoch
reviewed
Jul 22, 2026
tzimnoch
reviewed
Jul 22, 2026
tzimnoch
reviewed
Jul 22, 2026
tzimnoch
reviewed
Jul 22, 2026
Draw glyphs via CSS :before on spade/heart/diamond/club-suit so red coloring stays on the symbol and out of the markup. Co-authored-by: Cursor <cursoragent@cursor.com>
The fixture uses east_hearts "k" to exercise case normalization; make that intent visible in the test description. Co-authored-by: Cursor <cursoragent@cursor.com>
Collaborator
Author
|
Thanks, @tzimnoch, for another thorough review! |
tzimnoch
reviewed
Jul 22, 2026
tzimnoch
reviewed
Jul 22, 2026
tzimnoch
reviewed
Jul 22, 2026
Replace regex CSS unit tests with Playwright computed-style checks, and keep disabled or entered text fully opaque so a non-white background does not show through. Co-authored-by: Cursor <cursoragent@cursor.com>
Avoid a duplicate spade Ace so the card-count note tests exercise length, not invalid-card handling. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
CSS: before content is unreliable for screen readers and copy/paste; keep pip coloring via explicit .deck-card rules instead. Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Multiple UI improvements.
Deployed to https://tameware.com/adam/bridge/dds/dds_mvp.html