Conversation
Agent-Logs-Url: https://github.com/Hack23/blacktrigram/sessions/694fa256-057d-41ae-a379-79b844d9677b Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Agent-Logs-Url: https://github.com/Hack23/blacktrigram/sessions/694fa256-057d-41ae-a379-79b844d9677b Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Agent-Logs-Url: https://github.com/Hack23/blacktrigram/sessions/694fa256-057d-41ae-a379-79b844d9677b Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Agent-Logs-Url: https://github.com/Hack23/blacktrigram/sessions/694fa256-057d-41ae-a379-79b844d9677b Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Agent-Logs-Url: https://github.com/Hack23/blacktrigram/sessions/60c3ab2b-de9f-4a36-a6df-7d302578426a Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Agent-Logs-Url: https://github.com/Hack23/blacktrigram/sessions/f4c16388-42b9-402a-a6b1-1d2782590e54 Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Agent-Logs-Url: https://github.com/Hack23/blacktrigram/sessions/f4c16388-42b9-402a-a6b1-1d2782590e54 Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Dependency Reviewβ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
github-actions
bot
added
documentation
π― Accessibility Test ResultsWCAG 2.1 Level AA Compliance Tests
Components Tested
Test Framework
|
There was a problem hiding this comment.
Pull request overview
Fixes stance-specific attack animations resolving to the generic jab, and makes Combat/Training usable on portrait mobile by switching to orientation-aware arena sizing, bottom-band reservation, and portrait-specific HUD behavior.
Changes:
- Introduces
resolveTechniqueAnimation(technique)and updates Combat/Training callsites to preferanimationId/idbefore name/regex fallbacks. - Adds orientation-aware responsive layout logic (portrait detection with hysteresis, portrait-forced mobile branch, 3:4 arena in portrait, bottom-band reservation).
- Adds portrait-mobile UI refinements (portrait status strip, overlay intensity attenuation, short-viewport control bottom positioning) with regression-focused tests.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/responsiveOrientationConstants.ts | Adds shared portrait hysteresis + portrait mobile bottom-band reservation constants/helpers. |
| src/utils/mobileLayoutHelpers.ts | Makes mobile arena bounds orientation-aware (4:3 landscape, 3:4 portrait) and adds overflow guards. |
| src/utils/deviceDetection.ts | Minor optional-chaining cleanup for touch/pointer detection. |
| src/types/constants/layout.ts | Updates getMobileControlsBottom to be viewport-height aware (short viewport => reduced band). |
| src/types/constants/layout.test.ts | Adds unit tests for getMobileControlsBottom height thresholds/boundaries. |
| src/systems/animation/core/index.ts | Re-exports resolveTechniqueAnimation from animation core barrel. |
| src/systems/animation/core/TechniqueAnimationResolution.test.ts | Adds tests covering techniqueβanimation resolution and key regression cases. |
| src/systems/animation/core/AnimationRegistry.ts | Adds resolveTechniqueAnimation, English normalization, and reorders/extends fallback regex mapping. |
| src/systems/animation/README.md | Documents the new techniqueβanimation resolution entrypoint and fallback order. |
| src/components/screens/training/hooks/useTrainingLayout.ts | Adds isPortrait, portrait-forced mobile branching, and portrait bottom-band reservation for training layout. |
| src/components/screens/training/hooks/useTrainingLayout.test.ts | Updates tests for portrait/landscape aspect ratios and new orientation logic. |
| src/components/screens/training/TrainingScreen3D.tsx | Migrates to resolveTechniqueAnimation, portrait camera adjustment, hides side HUDs in portrait mobile, forwards viewport height for controls. |
| src/components/screens/training/README.md | Documents training responsive-orientation profiles and expected behavior. |
| src/components/screens/combat/hooks/useCombatLayout.ts | Adds isPortrait, portrait-forced mobile branching, portrait status strip reservation, and portrait bottom-band reservation for combat layout. |
| src/components/screens/combat/hooks/useCombatLayout.test.ts | Updates combat layout tests to validate portrait/landscape aspect behavior and on-screen bounds. |
| src/components/screens/combat/hooks/useCombatLayout.responsive.test.ts | Adds viewport-matrix regression tests ensuring arena remains visible and orientation-correct. |
| src/components/screens/combat/components/indicators/StaminaWarning.tsx | Adds intensityScale attenuation support for portrait-mobile overlay softening. |
| src/components/screens/combat/components/hud/index.ts | Exports the new CombatPortraitStatusStrip from HUD barrel. |
| src/components/screens/combat/components/hud/PlayerStateOverlayHtml.tsx | Adds intensityScale prop and cascades it to fullscreen effects with memo comparator update. |
| src/components/screens/combat/components/hud/MobileControlsWrapper.tsx | Adds optional viewportHeight prop and applies it to control bottom positioning. |
| src/components/screens/combat/components/hud/CombatPortraitStatusStrip.tsx | Adds a portrait-mobile compact two-player HP/stamina strip replacing hidden side HUDs. |
| src/components/screens/combat/components/hud/CombatPortraitStatusStrip.test.tsx | Tests portrait status strip rendering, clamping behavior, and positioning. |
| src/components/screens/combat/components/effects/intensityScale.test.tsx | Adds tests verifying intensityScale attenuates opacity/blur as intended. |
| src/components/screens/combat/components/effects/PainVignette.tsx | Adds intensityScale and applies it to vignette max opacity. |
| src/components/screens/combat/components/effects/ConsciousnessBlur.tsx | Adds intensityScale and applies it to blur + darkening intensity. |
| src/components/screens/combat/components/effects/BloodLossOverlayHtml.tsx | Adds intensityScale and updates memo comparator to include it. |
| src/components/screens/combat/README.md | Documents combat responsive-orientation profiles and portrait-specific rules. |
| src/components/screens/combat/CombatScreen3D.tsx | Migrates animation selection to resolveTechniqueAnimation, adds portrait camera tweaks, collapses side HUDs in portrait, adds portrait status strip, and passes viewport height for controls. |
| <div | ||
| role="progressbar" | ||
| aria-label={`Player ${side === "left" ? 1 : 2} stamina`} | ||
| aria-valuenow={Math.ceil(player.stamina)} | ||
| aria-valuemin={0} | ||
| aria-valuemax={player.maxStamina} | ||
| style={{ |
There was a problem hiding this comment.
Same issue as the HP bar: aria-valuenow uses the raw player.stamina while the fill is clamped to 0β100%. To keep the progressbar semantics valid for screen readers, clamp aria-valuenow to 0..player.maxStamina.
| @@ -78,12 +97,12 @@ export const StaminaWarning: React.FC<StaminaWarningProps> = ({ | |||
| inset: borderWidth, | |||
| pointerEvents: "none" as const, | |||
| border: `${borderWidth} solid ${warningColor}`, | |||
| boxShadow: `0 0 20px ${warningColor}`, | |||
| boxShadow: `0 0 ${Math.round(20 * safeScale)}px ${warningColor}`, | |||
| animation: `staminaFlash ${animationDuration}s ease-in-out infinite`, | |||
There was a problem hiding this comment.
The comment says intensityScale is clamped βso border always renders at its natural width, only the glow + inset/opacity fadesβ, but warningColorβs alpha is scaled by safeScale and used for the border color as well. With intensityScale=0, the border becomes fully transparent.
Either adjust the comment to match the behavior, or keep the border color at full alpha and apply intensityScale only to the glow/opacity so the border remains visible as described.
| // Calculate available space for the area | ||
| // Extra-small devices (<380px) use tighter margins for more screen real estate | ||
| const horizontalMargin = width < 380 ? 30 : 40; // 15px vs 20px per side | ||
| const availableHeight = height - topClearance - bottomClearance; | ||
| const availableWidth = width - horizontalMargin; | ||
| const availableHeight = Math.max( | ||
| 0, | ||
| height - topClearance - bottomClearance, | ||
| ); | ||
| const availableWidth = Math.max(0, width - horizontalMargin); |
There was a problem hiding this comment.
calculateMobileAreaBounds computes availableHeight from height - topClearance - bottomClearance, but returns y: yOffset. When callers pass yOffset > topClearance (which happens in the updated combat/training layout hooks), the function can oversize the arena because it assumes more vertical space than actually exists below yOffset, allowing yOffset + areaHeight to exceed height - bottomClearance.
Consider deriving the available height from the actual offset (e.g., use Math.max(topClearance, yOffset) for the top reservation, or compute availableHeight = height - yOffset - bottomClearance) so the returned bounds are guaranteed to fit given the chosen yOffset.
| [/rhythmic.?strike|리λ¬ν격/i, "rhythmic_strikes"], | ||
| // Heavenly/dragon fist names are stance-specific and should map to the | ||
| // proper Geon/Jin animations, not the generic jab/cross fallbacks. | ||
| [/heavenly.?fist|μ²κΆ|μ κΆ/i, "heaven_strike"], |
There was a problem hiding this comment.
The fallback regex [/heavenly.?fist|μ²κΆ|μ κΆ/i, "heaven_strike"] will also match the generic Korean term "μ κΆ" (straight punch/jab). Because this rule sits before the generic jab regex, calling getAnimationForTechnique("μ κΆ") would incorrectly resolve to heaven_strike instead of a jab animation.
Suggest narrowing this pattern to only the intended Heavenly Fist tokens (e.g. keep "μ²κΆ" / "heavenly fist"), or otherwise ensure "μ κΆ" continues to resolve to jab.
| [/heavenly.?fist|μ²κΆ|μ κΆ/i, "heaven_strike"], | |
| [/heavenly.?fist|μ²κΆ/i, "heaven_strike"], |
| role="progressbar" | ||
| aria-label={`Player ${side === "left" ? 1 : 2} health`} | ||
| aria-valuenow={Math.ceil(player.health)} | ||
| aria-valuemin={0} | ||
| aria-valuemax={player.maxHealth} | ||
| style={{ |
There was a problem hiding this comment.
aria-valuenow is set from the raw player.health value, but the visual fill is clamped to 0β100%. If player.health goes negative or above maxHealth (as your own test covers for the fill), the progressbar ARIA values can fall outside [aria-valuemin, aria-valuemax], which is invalid for assistive tech.
Clamp aria-valuenow to the same bounds used for the fill (0..maxHealth).
|
@copilot apply changes based on the comments in this thread |
πΈ Automated UI Screenshotsπ Screenshots Captured (8)
π¦ Download Screenshotsπ₯ Download all screenshots from workflow artifacts
π€ Generated by Playwright automation |
β¦pha, mobile bounds overflow, μ κΆ regex Agent-Logs-Url: https://github.com/Hack23/blacktrigram/sessions/ed460b88-b88e-4dbe-85d6-0291f2f5bd0c Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
|
Addressed the 5 review comments in ad72e80:
|
π― Accessibility Test ResultsWCAG 2.1 Level AA Compliance Tests
Components Tested
Test Framework
|
| typeof window !== "undefined" && | ||
| window.matchMedia && | ||
| window.matchMedia("(pointer: coarse)").matches | ||
| window.matchMedia?.("(pointer: coarse)").matches |
There was a problem hiding this comment.
window.matchMedia?.("(pointer: coarse)").matches is not fully guarded: if matchMedia is undefined, the optional call returns undefined and the subsequent .matches access will throw. Use optional chaining on the result as well (e.g. window.matchMedia?.("(pointer: coarse)")?.matches).
| window.matchMedia?.("(pointer: coarse)").matches | |
| window.matchMedia?.("(pointer: coarse)")?.matches |
| const healthPercent = Math.max( | ||
| 0, | ||
| Math.min(100, (player.health / player.maxHealth) * 100), | ||
| ); | ||
| const staminaPercent = Math.max( | ||
| 0, | ||
| Math.min(100, (player.stamina / player.maxStamina) * 100), | ||
| ); |
There was a problem hiding this comment.
healthPercent/staminaPercent compute player.health / player.maxHealth and player.stamina / player.maxStamina. If maxHealth/maxStamina is 0 (edge case already covered elsewhere in the repo), this yields Infinity/NaN and can produce incorrect widths like 100% or NaN%. Guard by treating non-positive maxima as 0% (or another explicit fallback) before the division.
πΈ Automated UI Screenshotsπ Screenshots Captured (8)
π¦ Download Screenshotsπ₯ Download all screenshots from workflow artifacts
π€ Generated by Playwright automation |
|
@copilot apply changes based on the comments in this thread and investigate and improve Test and Report / unit-tests (pull_request) |
β¦ perf threshold Agent-Logs-Url: https://github.com/Hack23/blacktrigram/sessions/704dc31c-ea76-4f0d-b984-39deb0c910d7 Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Addressed in e56fb69:
|
π― Accessibility Test ResultsWCAG 2.1 Level AA Compliance Tests
Components Tested
Test Framework
|
πΈ Automated UI Screenshotsπ Screenshots Captured (8)
π¦ Download Screenshotsπ₯ Download all screenshots from workflow artifacts
π€ Generated by Playwright automation |
Plan
Part 1 β Animation resolution β
Part 2 β Responsive Combat + Training layouts β
Part 3 β Initial UI/UX pass β
Part 4 β Follow-up UI/UX pass β
Part 5 β Address initial code-review feedback β
Part 6 β Follow-up code-review feedback + CI flake β
deviceDetection.tsβ optional-chain the.matchesaccess as well (matchMedia?.(β¦)?.matches) so an undefinedmatchMediacan't throw when the optional call returnsundefined.CombatPortraitStatusStripβ guardhealthPercent/staminaPercentagainst non-positivemaxHealth/maxStamina. Treat<= 0maxima as 0 % instead of dividing (which would yieldInfinity/NaNand render invalid widths).AnimationTransitions.stance.test.tsβ bump the "retrieve transitions quickly" micro-benchmark threshold from 10 ms to 50 ms. 1000 Map lookups in <10 ms is flaky on shared CI runners (observed 13.58 ms on the failingunit-testsjob); 50 ms still catches real regressions but absorbs runner jitter.AnimationTransitions.stance,CombatPortraitStatusStrip,deviceDetectionβ 75/75);npx tsc -b+npx eslintclean.