feat: discoverable priority-yield auto-pass UX (CR 117.3d)#5167
Merged
Conversation
….3d) The priority-yield auto-pass control shipped in #5132 was reachable only by long-pressing a triggered stack entry — an undiscoverable gesture that also collided with the mobile card-preview long-press (both fired on the same triggered entry, so trigger cards could no longer be previewed). Replace it with an always-visible tap button on triggered-ability entries that toggles the existing yield menu, and restore long-press to a single uniform meaning (inspect-and-pin) for every entry. The button sits on the center-right edge — the one card zone clear of the status badges, chip row, and ability overlay — and turns amber (bell-off icon) when a yield already stands for the trigger. Pure display/dispatch: the engine still owns all yield state via SetPriorityYield / priority_yields.
…m use (CR 117.1d) auto_pass_recommended held frontend priority at every window whenever the player controlled a sacrifice-for-mana permanent (Eldrazi Spawn, Krark-Clan Ironworks, Phyrexian Altar), even when the mana could pay for nothing and the sacrifice triggered nothing. On an opponent's turn with an uncastable hand the player was asked for priority endlessly (repro: 8 Eldrazi Spawn, 3 sorceries in hand, opponent trigger on the stack, legal_actions == [PassPriority]). Narrow the issue-#544 sac-for-mana hold: it now blocks auto-pass only when the sacrifice or its mana enables a concrete follow-up — a feasibly castable spell (already handled by the #4388 castability rungs above), a mana-costed non-mana activated ability (case 2), or a leaves-battlefield/dies/sacrifice trigger (case 3, aristocrats/altars). A bare sac-for-mana with none of these auto-passes. The narrowing lives entirely in auto_pass_recommended. has_meaningful_priority_action is kept byte-identical (extracted into flat_actions_have_meaningful_priority + has_activatable_sacrifice_for_mana, then recomposed) because the CR 732.5 loop-shortcut firewall depends on it still counting a sacrifice as a board change. mana_sources.rs is untouched. Cases 2 and 3 are conservative presence checks that err toward holding, so no real play is ever silently auto-passed; a precise sac->activate affordability check is a documented follow-up. 'Counter unless pay {1}' is unaffected — that payment is a WaitingFor::UnlessPayment prompt at resolution, never a priority window (regression test V7). CR 605.3a + 603.6 + 603.6c + 117.1d + 118.12a.
… open The always-visible yield control on triggered stack entries was a bare dark icon that read as decoration and was easy to miss. Replace it with a high-contrast labeled pill (indigo resting / amber when a yield stands) carrying a compact 'Auto-pass' label so its purpose is legible without hovering. Add menuButtonShort / menuButtonShortActive across all 7 locales. Also clear the hovered/sticky card preview when the yield menu opens and gate the hover handler while it's open, so the preview no longer lingers on top of the menu while the player reads the options (mouseenter never re-fires or clears as the pointer moves onto the menu's child elements).
…ing yields into a compact chip (CR 117.3d) - PopoverMenu: seal onPointerDown as well as onClick on the portal container so a menu interaction never leaks through the React tree to the card's useLongPress (which was firing the card preview and leaving the menu open — the "options do nothing" bug). Bubble-phase only, so the capture-phase outside-click dismissal is unaffected. - PriorityYieldList: collapse the unbounded vertical list into one fixed-footprint amber "Auto-passing N" chip that opens the revocable list inside the now-sealed PopoverMenu, keeping the action rail height constant regardless of yield count. - FullControlToggle: shorten the visible label to "Control" (aria-pressed + descriptive aria-label preserve toggle semantics; tooltip elaborates), placed on the same row as the chip. - Extract the shared YieldMuteIcon glyph; add PopoverMenu and PriorityYieldList regression tests (the PopoverMenu test fails without the pointer-event seal).
Contributor
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
Parse changes introduced by this PR · 0 card(s), 0 signature(s) (baseline: main
|
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
1 participant
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.
Summary
Priority-yield ("auto-pass") UX for CR 117.3d, building on the engine mechanism from #5132. Four commits:
feat(client): discoverable tap affordance for priority yields — the yield control was reachable only by long-pressing a triggered stack entry (undiscoverable, and it collided with the mobile card-preview long-press). Replaced with an always-visible tap button on triggered-ability entries; long-press is restored to a single uniform meaning (inspect-and-pin).
fix(engine): auto-pass past bare sacrifice-for-mana with no downstream use (CR 117.1d) —
auto_pass_recommendedheld priority at every window whenever the player controlled a sac-for-mana permanent (Eldrazi Spawn, Krark-Clan Ironworks, Phyrexian Altar), even when the mana could pay for nothing and the sacrifice triggered nothing (endless priority on an opponent's turn with an uncastable hand). The Krark-Clan Ironworks — The game auto-passes in this board state when it shouldn't, because I can activate Krark-Clan Ir… #544 hold is narrowed to block auto-pass only when the sacrifice/mana enables a concrete follow-up: a feasibly castable spell, a mana-costed non-mana activated ability, or a leaves-battlefield/dies/sacrifice trigger.has_meaningful_priority_actionis kept byte-identical (extracted + recomposed) because the CR 732.5 loop-shortcut firewall depends on it still counting a sacrifice as a board change. This is the auto-pass/AI-adjacent change — see the commit body for the full case analysis and the V7Counter unless pay {1}regression note.fix(stack): make yield affordance discoverable and dismiss preview on open — polish on the tap affordance; dismisses the card preview when the yield menu opens.
fix(client): make priority-yield menu options work and collapse standing yields into a compact chip — the core interaction fix:
PopoverMenunow sealsonPointerDown(not justonClick) on its portal container, so a menu interaction no longer leaks through the React tree to the card'suseLongPress(which was firing the card preview and leaving the menu open — the "options do nothing" bug). Standing yields collapse into one fixed-footprint amber "Auto-passing N" chip that opens the revocable list;FullControlToggleshortened to "Control" (ARIA preserves toggle semantics) on the same row.Testing
PopoverMenuregression test (fails without the pointer-event seal),PriorityYieldList(5 cases), updatedStackEntrytests, i18n parity across all 7 locales.check-frontendgreen (tsc + eslint).Notes
Two interleaved report-card commits on local
mainare intentionally excluded — they ship via their ownship/report-card-nudgeandship/report-picker-polishbranches.