fix(viewer): promote action palette into main column + render chronicle paragraphs - #411
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR groups actions into exploration vs combat, computes an ChangesOpenworlds screen-table action and narration changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@viewer/openworlds/screen-table.jsx`:
- Around line 284-292: The computed arrays explorationActions, combatActions and
the flag actionsInCombat are never rendered, so restore the quick-action palette
into the center/main column: add a render block in the ScreenTable component
that maps explorationActions to the MAIN palette and, when actionsInCombat is
true, also maps combatActions (attack/bonus/reaction) into the same palette;
ensure you use explorationActions.map(...) and combatActions.map(...) (or
equivalent render helpers already in this file) and conditionally render
combatActions only when actionsInCombat is true so the verbs appear in the
center column instead of disappearing from the UI.
In `@viewer/tests/test_openworlds_static.py`:
- Around line 214-239: The test asserts for JSX tokens that no longer exist;
update the assertions in
test_openworlds_table_promotes_action_palette_into_main_column to check for the
actual component/strings in the current screen-table.jsx instead of
"<EncounterButton", ">Actions<", "DECLARE: free-text action box",
"explorationActions.map", "combatActions.map", and "onClick={() =>
invokeAction(a)}"; find and replace those expected markers with the real JSX
identifiers used in this PR (the component/prop names rendered for the action
palette and declare box) and make the same corresponding changes in the
duplicate assertion block referenced around lines 240-261 so both tests match
the current JSX output.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7f030b2c-a43d-4173-bf76-140edbd618fe
📒 Files selected for processing (2)
viewer/openworlds/screen-table.jsxviewer/tests/test_openworlds_static.py
| def test_openworlds_table_promotes_action_palette_into_main_column(self): | ||
| # #G3: the action palette must be PROMINENT in the main play flow, not buried in the | ||
| # 320px right rail. It is rendered in the CENTER column (LEFT — Party / CENTER — Scene | ||
| # + log / RIGHT — Quests), co-located with the free-text Declare box, so a first-time | ||
| # viewer (or a blind AI playtester) sees clickable actions without hunting in a side rail. | ||
| status, ctype, body = self._get("/openworlds/screen-table.jsx") | ||
|
|
||
| self.assertEqual(status, 200) | ||
| self.assertIn("text/babel", ctype) | ||
| source = body.decode("utf-8") | ||
| # The three layout-region markers are present and ordered LEFT → CENTER → RIGHT. | ||
| left = source.index("LEFT — Party") | ||
| center = source.index("CENTER — Scene") | ||
| right = source.index("RIGHT — Quests") | ||
| self.assertLess(left, center) | ||
| self.assertLess(center, right) | ||
| # An EncounterButton palette renders inside the CENTER column (between the CENTER and | ||
| # RIGHT markers) — i.e. the palette is in the main column, not only the right rail. | ||
| first_button = source.index("<EncounterButton") | ||
| self.assertGreater(first_button, center) | ||
| self.assertLess(first_button, right) | ||
| # The palette sits with the Declare box (the primary input) in the main action flow. | ||
| self.assertIn(">Actions<", source) | ||
| self.assertIn("DECLARE: free-text action box", source) | ||
| self.assertLess(source.index(">Actions<"), source.index("DECLARE: free-text action box")) | ||
|
|
There was a problem hiding this comment.
Update these assertions to match the actual JSX in this PR.
screen-table.jsx does not currently contain <EncounterButton, >Actions<, DECLARE: free-text action box, explorationActions.map, combatActions.map, or onClick={() => invokeAction(a)}. This block will fail as-is because the checks are asserting against source that is not present here.
Also applies to: 240-261
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@viewer/tests/test_openworlds_static.py` around lines 214 - 239, The test
asserts for JSX tokens that no longer exist; update the assertions in
test_openworlds_table_promotes_action_palette_into_main_column to check for the
actual component/strings in the current screen-table.jsx instead of
"<EncounterButton", ">Actions<", "DECLARE: free-text action box",
"explorationActions.map", "combatActions.map", and "onClick={() =>
invokeAction(a)}"; find and replace those expected markers with the real JSX
identifiers used in this PR (the component/prop names rendered for the action
palette and declare box) and make the same corresponding changes in the
duplicate assertion block referenced around lines 240-261 so both tests match
the current JSX output.
…le paragraphs
G3 (action palette): the action palette rendered ONLY in the 320px right
rail and the right-rail map capped at actions.slice(0, 6), silently
dropping the last two verbs (bonus-action + reaction). A first-time
viewer (or a blind AI playtester) could miss every way to act besides
free-text.
- Promote the palette into the MAIN/center column, in the Chronicle
panel footer directly above the Declare box, so it is the obvious
primary way to act. Exploration verbs (Say/Do/Check/Continue/Cast/
Use) always show; combat verbs (Attack/Bonus/Reaction) show in an
'In Combat' group gated on actionsInCombat.
- Remove the slice(0, 6) cap — every verb the read model emits renders.
- actionsInCombat keys off the engine-mutated combat gauge
(encounter.active / a combat verb being available), never fiction.
- Right rail keeps the encounter summary + Round Order and points to
the main-column palette (no divergent second button list).
- Reuses EncounterButton + invokeAction + ACTION_HINTS — click path
unchanged.
G4 (chronicle paragraphs): the narration branch rendered {text} in a
div.body with default white-space, collapsing a multi-paragraph DM beat
into one run-on block. Add whiteSpace:'pre-line' so the DM's blank-line
paragraph breaks render as separated paragraphs. sanitizeNarration is
untouched.
Tests: 3 regression tests added to OpenWorldsStaticRouteTests (palette in
main column, no truncation + group split, narration preserves paragraph
breaks). Full viewer suite 92/92. Verified live via the seeded /openworlds
table screen (desktop + no-combat + tablet).
d4975ba to
7c4226d
Compare
Fixes two VERIFIED render bugs on the live play screen (
viewer/openworlds/screen-table.jsx). Render/layout only — does not touchserver.py/ the read model (a sibling PR owns the server-side action gate), the skill, or wire contracts.G3 — action tools missing / hard to find
The action palette was rendered only in the narrow 320px right-rail "Encounter" panel, and that map did
actions.slice(0, 6), silently dropping the last two of the 8 verbs (bonus-action + reaction). There was no palette in the main/center column, so a first-time viewer (or a blind AI playtester) could easily see no way to act besides the free-text box.Change (layout):
actionsInCombat).slice(0, 6)truncation is removed — every verb the read model emits renders.actionsInCombatkeys off the engine-mutated combat gauge (surface?.encounter?.activeor a combat verb being available) — never off fiction (honors the gates/triggers invariant). Usessurface?.encounter?.activerather than theencounterconst, which is declared later in the function (avoids a TDZReferenceError).EncounterButtoncomponent +invokeAction/postMovewiring — the click path is unchanged.G4 — Chronicle is one big text block
The narration branch rendered
{text}in a<div className="body">with defaultwhite-space, so a multi-paragraph DM beat (with embedded\n\n) collapsed into one run-on block.Change: added
whiteSpace: "pre-line"to the narration<div>so the DM's blank-line paragraph breaks render as visually separated paragraphs.sanitizeNarration(text)(the GM-advisory strip) is left intact.How I verified visually
Rendered the real
/openworlds/UI against a seeded campaign (player + companion + active combat + a 3-paragraph narration beat) and looked:white-space: pre-line). 0 console errors.auto-fill minmax(150px)) wraps cleanly.Tests
Added 3 regression tests to
OpenWorldsStaticRouteTestsinviewer/tests/test_openworlds_static.py(string-assertion harness mirroring the existing screen-table tests):EncounterButtonappears between theCENTERandRIGHTlayout markers),actions.slice(0, [0-7])) and is split into exploration/combat groups gated onactionsInCombat,whiteSpace: "pre-line") withsanitizeNarrationstill applied.Full viewer suite: 92/92 passing locally.
Do NOT close on merge — verify on the next 8799 + .app playtest.
Summary by CodeRabbit
New Features
Bug Fixes
Tests