Closes #503 feat:Added fireMouse and click helpers#667
Open
Yazhini74 wants to merge 3 commits into
Open
Conversation
github-actions
Bot
added
type:docs
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PR adds mouse event simulation helpers to the testing harness and extends the theme system. ChangesMouse Event Testing Support
Theme System Extensions
Sequence Diagram(s)sequenceDiagram
participant TestInstance
participant Widget
participant Renderer
participant ScreenBuffer
TestInstance->>Widget: hit-test coords -> select target
TestInstance->>Widget: call handleMouse(event) or events.emit('mouse', event)
Widget->>Renderer: update state (if any)
Renderer->>ScreenBuffer: re-render root and redraw buffer
🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labelstype:feature, area:testing, area:themes Suggested reviewers
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 unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
index.ts (1)
1-10: Consider relocating this example and adding execution infrastructure.The code itself is functionally correct, but the placement and execution setup could be improved:
Location inconsistency: This example is at the repository root, while other examples (like
examples/dashboard) follow a structured pattern with their own folders andpackage.jsonfiles.Missing ESM configuration: The file uses top-level
await(line 10), which requires either Bun or Node with"type": "module"inpackage.json. The rootpackage.jsondoesn't specify this.No execution script: There's no script in the root
package.jsonto run this file, making it unclear how users should execute it.Consider either:
- Moving this to
examples/hello-world/with its ownpackage.jsonand README following theexamples/dashboardpattern, or- Adding a
"demo"or"example"script to rootpackage.json(e.g.,"demo": "bun index.ts") and adding"type": "module"if Node compatibility is desired.🤖 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 `@index.ts` around lines 1 - 10, This example index.ts is placed at the repo root and uses top-level await (await app.mount()) but lacks ESM/run infrastructure; move the example into a new examples/hello-world/ directory (mirroring examples/dashboard) with its own package.json and README, or update root package.json to add "type": "module" and a run script (e.g., "demo") to execute index.ts; ensure the new package.json scripts reference the same App/root Box usage (root, App, app.mount) so the example runs with a compatible runtime (Node ESM or Bun).
🤖 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 `@packages/testing/src/render.test.tsx`:
- Around line 230-263: Add two tests to the existing suite using the same
render/screen/Clickable pattern: (1) a "move" test that calls
screen.fireMouse(x,y,{type: 'move'}) against a Clickable instance that exposes a
hover/hovered state (or add a transient hovered rendering to Clickable) and
assert the render output reflects hover (e.g., contains "HOVER") without
toggling clicked; (2) a handlerless in-bounds no-op test that renders a plain
widget/component with no mouse handlers (not Clickable), calls screen.fireMouse
for coordinates inside bounds and asserts no state changes and no exceptions
(e.g., original component output unchanged and no clickable.clicked). Use the
same helpers and identifiers: fireMouse, Clickable, render, screen, and click
for consistency.
- Around line 221-227: Change Clickable._renderSelf to match Widget's abstract
signature by importing Screen from `@termuijs/core` and replacing the loose any
type: update the method signature protected _renderSelf(screen: Screen): void
and add the import for Screen; ensure handleMouse remains the same (this.clicked
= true; this.markDirty()) and that screen.writeString usage continues to work
with the Screen type.
In `@packages/testing/src/render.ts`:
- Line 55: The public API currently types fireMouse as init?:
Partial<MouseEvent> and internally emits 'mousedown'/'mouseup'; change the
signature of fireMouse (and any public click wrapper) to accept init?:
MouseEventInit where MouseEventInit.type is a union 'down' | 'up' | 'move'
(default 'down') and init.button defaults to 'left'; inside the implementation
map these logical types to DOM event names ('down' -> 'mousedown', 'up' ->
'mouseup', 'move' -> 'mousemove') and ensure callers using { type: 'move' }
work, including updating the related call sites that construct events in the
methods referenced (fireMouse, click) to use the new MouseEventInit contract and
apply defaults when type/button are omitted.
- Around line 327-329: Replace the casted call "(target as
any).handleMouse(event)" with a safe runtime lookup/call: get the property by
name from target (e.g. const fn = (target as unknown as Record<string,
unknown>)['handleMouse']), check if typeof fn === 'function', and then invoke it
with fn.call(target, event) to preserve the receiver; this removes the use of
"as any" while still allowing optional handleMouse invocation on target.
In `@packages/tss/src/index.ts`:
- Around line 26-30: The package export list is missing gruvboxTheme causing
root imports to fail; update the re-export in index.ts to include gruvboxTheme
from './named-themes.js' alongside the other symbols (e.g., draculaTheme,
NAMED_THEMES, getNamedTheme) so the documented public API matches the actual
exports.
---
Nitpick comments:
In `@index.ts`:
- Around line 1-10: This example index.ts is placed at the repo root and uses
top-level await (await app.mount()) but lacks ESM/run infrastructure; move the
example into a new examples/hello-world/ directory (mirroring
examples/dashboard) with its own package.json and README, or update root
package.json to add "type": "module" and a run script (e.g., "demo") to execute
index.ts; ensure the new package.json scripts reference the same App/root Box
usage (root, App, app.mount) so the example runs with a compatible runtime (Node
ESM or Bun).
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 731e95bf-5a33-4ae5-903e-74baadb5b91f
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
examples/dashboard/package.jsonindex.tspackage.jsonpackages/testing/src/render.test.tsxpackages/testing/src/render.tspackages/tss/README.mdpackages/tss/src/index.tspackages/tss/src/named-themes.tspackages/tss/src/themes.test.tspackages/tss/src/themes.ts
Comment on lines
+230
to
+263
| it("fireMouse invokes handleMouse and re-renders", () => { | ||
| const clickable = new Clickable() | ||
| const App = () => clickable | ||
| const screen = render(<App />) | ||
|
|
||
| expect(screen.renderToString()).toContain("IDLE") | ||
|
|
||
| screen.fireMouse(0, 0) | ||
|
|
||
| expect(screen.renderToString()).toContain("CLICKED") | ||
| }) | ||
|
|
||
| it("click(x, y) simulates a full click", () => { | ||
| const clickable = new Clickable() | ||
| const App = () => clickable | ||
| const screen = render(<App />) | ||
|
|
||
| expect(screen.renderToString()).toContain("IDLE") | ||
|
|
||
| screen.click(0, 0) | ||
|
|
||
| expect(screen.renderToString()).toContain("CLICKED") | ||
| }) | ||
|
|
||
| it("fireMouse is a no-op if out of bounds", () => { | ||
| const clickable = new Clickable() | ||
| const App = () => clickable | ||
| const screen = render(<App />) | ||
|
|
||
| screen.fireMouse(50, 50) | ||
|
|
||
| expect(clickable.clicked).toBe(false) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Add tests for type: 'move' and handlerless in-bounds no-op.
Current suite validates down/up and out-of-bounds, but it misses two explicit acceptance cases: fireMouse(..., { type: 'move' }) hover behavior without click, and safe no-op when coordinates are in bounds but no handler-bearing widget should handle the event.
🤖 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 `@packages/testing/src/render.test.tsx` around lines 230 - 263, Add two tests
to the existing suite using the same render/screen/Clickable pattern: (1) a
"move" test that calls screen.fireMouse(x,y,{type: 'move'}) against a Clickable
instance that exposes a hover/hovered state (or add a transient hovered
rendering to Clickable) and assert the render output reflects hover (e.g.,
contains "HOVER") without toggling clicked; (2) a handlerless in-bounds no-op
test that renders a plain widget/component with no mouse handlers (not
Clickable), calls screen.fireMouse for coordinates inside bounds and asserts no
state changes and no exceptions (e.g., original component output unchanged and
no clickable.clicked). Use the same helpers and identifiers: fireMouse,
Clickable, render, screen, and click for consistency.
| * Simulate a mouse event at the given coordinates. | ||
| * Hit-tests the widget tree and dispatches to the top-most widget. | ||
| */ | ||
| fireMouse(x: number, y: number, init?: Partial<MouseEvent>): void; |
There was a problem hiding this comment.
Align fireMouse/click with the agreed MouseEventInit contract.
Line 55 currently exposes init?: Partial<MouseEvent>, and Lines 312/347-348 use 'mousedown'/'mouseup'. The PR contract requires a public MouseEventInit with 'down' | 'up' | 'move' (default 'down') and button default 'left'. As-is, callers using { type: 'move' } per acceptance criteria won’t match the public API.
Suggested patch
+export interface MouseEventInit {
+ type?: 'down' | 'up' | 'move';
+ button?: 'left' | 'right' | 'middle';
+}
+
export interface TestInstance {
@@
- fireMouse(x: number, y: number, init?: Partial<MouseEvent>): void;
+ fireMouse(x: number, y: number, init?: MouseEventInit): void;
@@
- fireMouse(x: number, y: number, init?: Partial<MouseEvent>) {
+ fireMouse(x: number, y: number, init?: MouseEventInit) {
// Normalize the mouse event
const event: MouseEvent = {
x,
y,
- type: init?.type ?? 'mousedown',
+ type: init?.type === 'up'
+ ? 'mouseup'
+ : init?.type === 'move'
+ ? 'mousemove'
+ : 'mousedown',
button: init?.button ?? 'left',
};
@@
click(x: number, y: number) {
- this.fireMouse(x, y, { type: 'mousedown' });
- this.fireMouse(x, y, { type: 'mouseup' });
+ this.fireMouse(x, y, { type: 'down' });
+ this.fireMouse(x, y, { type: 'up' });
},Also applies to: 307-313, 346-348
🤖 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 `@packages/testing/src/render.ts` at line 55, The public API currently types
fireMouse as init?: Partial<MouseEvent> and internally emits
'mousedown'/'mouseup'; change the signature of fireMouse (and any public click
wrapper) to accept init?: MouseEventInit where MouseEventInit.type is a union
'down' | 'up' | 'move' (default 'down') and init.button defaults to 'left';
inside the implementation map these logical types to DOM event names ('down' ->
'mousedown', 'up' -> 'mouseup', 'move' -> 'mousemove') and ensure callers using
{ type: 'move' } work, including updating the related call sites that construct
events in the methods referenced (fireMouse, click) to use the new
MouseEventInit contract and apply defaults when type/button are omitted.
Comment on lines
+327
to
+329
| if (typeof (target as any).handleMouse === 'function') { | ||
| (target as any).handleMouse(event); | ||
| } else { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant section around the reported lines
sed -n '280,380p' packages/testing/src/render.ts | nl -ba
# Find all uses of "as any" in the repo (to see if this is an isolated case)
rg -n "as any" packages/testing/src/render.ts packages/testing/src -S || true
# Inspect the declaration/type of "target" in this file (best-effort search)
rg -n "target" packages/testing/src/render.ts | head -n 80
# Show any type assertions in the file to compare with guidelines
rg -n "as [A-Za-z_][A-Za-z0-9_]*" packages/testing/src/render.ts -S || trueRepository: Karanjot786/TermUI
Length of output: 105
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/testing/src/render.ts"
# Show the relevant section around the reported lines with line numbers (no nl dependency)
sed -n '280,380p' "$FILE" | awk '{print (NR+279) "\t" $0}'
# Search for "as any" in the file and nearby scope
rg -n "as any" "$FILE" -S || true
rg -n "as any" packages/testing/src -S || true
# Inspect how `target` is typed/declared in this file (best-effort)
rg -n "target" "$FILE" | head -n 120
# Inspect other type assertions in this file
rg -n " as [A-Za-z_][A-Za-z0-9_]*" "$FILE" -S || trueRepository: Karanjot786/TermUI
Length of output: 9009
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find all occurrences of handleMouse
rg -n "handleMouse" packages -S
# Locate Widget type definition (interface/class) used by packages/testing/src/render.ts
rg -n "export (interface|type|class) Widget|interface Widget|type Widget|class Widget" packages -S
# Open the most relevant Widget definition file(s)
# (Use head/tail to avoid huge outputs; adjust if multiple hits)
FILES=$(rg -l "interface Widget|type Widget|class Widget" packages -S | tr '\n' ' ')
echo "Widget-related files: $FILES" | head -n 5
for f in $FILES; do
echo "---- $f ----"
# print a small window around "handleMouse" if present; else print first ~120 lines
if rg -n "handleMouse" "$f" -S >/dev/null; then
# show around handleMouse
line=$(rg -n "handleMouse" "$f" -S | head -n1 | cut -d: -f1)
start=$((line-20)); end=$((line+20))
sed -n "${start},${end}p" "$f" | nl -ba | head -n 120
else
sed -n '1,160p' "$f" | nl -ba | head -n 160
fi
doneRepository: Karanjot786/TermUI
Length of output: 1043
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE_WIDGET="packages/widgets/src/base/Widget.ts"
FILE_TEST="packages/testing/src/render.test.tsx"
FILE_RENDER="packages/testing/src/render.ts"
echo "---- handleMouse occurrences ----"
rg -n "handleMouse" "$FILE_WIDGET" "$FILE_TEST" "$FILE_RENDER" -S || true
echo
echo "---- packages/widgets/src/base/Widget.ts (around handleMouse) ----"
if rg -n "handleMouse" "$FILE_WIDGET" -S >/dev/null; then
line=$(rg -n "handleMouse" "$FILE_WIDGET" -S | head -n1 | cut -d: -f1)
start=$((line-40)); end=$((line+60))
sed -n "${start},${end}p" "$FILE_WIDGET" | awk '{print (NR+ '"$start"'-1)"\t"$0}'
else
sed -n '1,220p' "$FILE_WIDGET" | awk '{print NR"\t"$0}'
fi
echo
echo "---- packages/testing/src/render.test.tsx (around handleMouse) ----"
line=$(rg -n "handleMouse" "$FILE_TEST" -S | head -n1 | cut -d: -f1)
start=$((line-40)); end=$((line+80))
sed -n "${start},${end}p" "$FILE_TEST" | awk '{print (NR+ '"$start"'-1)"\t"$0}'
echo
echo "---- packages/testing/src/render.ts (full fireMouse block) ----"
# print from fireMouse start to end of block
start=$(rg -n "fireMouse\\(" "$FILE_RENDER" -S | head -n1 | cut -d: -f1)
# rough end
end=$((start+60))
sed -n "${start},${end}p" "$FILE_RENDER" | awk '{print (NR+ '"$start"'-1)"\t"$0}'Repository: Karanjot786/TermUI
Length of output: 11522
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/testing/src/render.ts"
# Show the exact fireMouse section (reported area)
sed -n '300,360p' "$FILE" | cat -n
# Also show surrounding helper types/signatures in the same file (to understand MouseEvent type, target types, etc.)
sed -n '1,120p' "$FILE" | cat -n
# Show Widget/MouseEvent imports at top (avoid whole file)
sed -n '1,70p' "$FILE" | cat -nRepository: Karanjot786/TermUI
Length of output: 9907
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate eslint / tsconfig configs that might enforce no-explicit-any / no-explicit-any in ts
ls -la
rg -n "no-explicit-any|explicit-any|ban-types|`@typescript-eslint/no-explicit-any`" -S .eslintrc* .eslint* eslint.* package.json tsconfig.json tsconfig.* 2>/dev/null || true
# Show relevant tsconfig settings if present
if [ -f tsconfig.json ]; then
echo "---- tsconfig.json ----"
cat tsconfig.json
fi
# Search for tsconfig that might be used by packages
for f in $(ls -1 **/tsconfig*.json 2>/dev/null || true); do
echo "---- $f ----"
rg -n "noImplicitAny|strict" "$f" || true
done
# Search for eslint config in packages
rg -n "no-explicit-any|`@typescript-eslint/no-explicit-any`" -S packages . 2>/dev/null || trueRepository: Karanjot786/TermUI
Length of output: 2940
Remove as any from the mouse dispatch path.
packages/widgets/src/base/Widget.ts doesn’t declare handleMouse, so packages/testing/src/render.ts currently uses (target as any) to call an optional method. Replace this with a safe runtime lookup/call that avoids as any.
Suggested patch
- if (typeof (target as any).handleMouse === 'function') {
- (target as any).handleMouse(event);
+ const handleMouse = Reflect.get(target, 'handleMouse');
+ if (typeof handleMouse === 'function') {
+ handleMouse.call(target, event);
} else {
target.events.emit('mouse', event);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (typeof (target as any).handleMouse === 'function') { | |
| (target as any).handleMouse(event); | |
| } else { | |
| const handleMouse = Reflect.get(target, 'handleMouse'); | |
| if (typeof handleMouse === 'function') { | |
| handleMouse.call(target, event); | |
| } else { |
🤖 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 `@packages/testing/src/render.ts` around lines 327 - 329, Replace the casted
call "(target as any).handleMouse(event)" with a safe runtime lookup/call: get
the property by name from target (e.g. const fn = (target as unknown as
Record<string, unknown>)['handleMouse']), check if typeof fn === 'function', and
then invoke it with fn.call(target, event) to preserve the receiver; this
removes the use of "as any" while still allowing optional handleMouse invocation
on target.
Comment on lines
26
to
30
| export { | ||
| draculaTheme, nordTheme, catppuccinTheme, monokaiTheme, | ||
| solarizedTheme, tokyoNightTheme, oneDarkTheme, highContrastTheme, | ||
| solarizedTheme, solarizedLightTheme, tokyoNightTheme, oneDarkTheme, highContrastTheme, | ||
| NAMED_THEMES, getNamedTheme, | ||
| } from './named-themes.js'; |
There was a problem hiding this comment.
Re-export gruvboxTheme to keep the documented public API consistent.
gruvboxTheme exists in packages/tss/src/named-themes.ts and is documented in packages/tss/README.md (Line 106), but it’s missing from this package-level export list, so root imports for it fail.
Proposed fix
export {
draculaTheme, nordTheme, catppuccinTheme, monokaiTheme,
- solarizedTheme, solarizedLightTheme, tokyoNightTheme, oneDarkTheme, highContrastTheme,
+ solarizedTheme, solarizedLightTheme, tokyoNightTheme, oneDarkTheme, gruvboxTheme, highContrastTheme,
NAMED_THEMES, getNamedTheme,
} from './named-themes.js';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export { | |
| draculaTheme, nordTheme, catppuccinTheme, monokaiTheme, | |
| solarizedTheme, tokyoNightTheme, oneDarkTheme, highContrastTheme, | |
| solarizedTheme, solarizedLightTheme, tokyoNightTheme, oneDarkTheme, highContrastTheme, | |
| NAMED_THEMES, getNamedTheme, | |
| } from './named-themes.js'; | |
| export { | |
| draculaTheme, nordTheme, catppuccinTheme, monokaiTheme, | |
| solarizedTheme, solarizedLightTheme, tokyoNightTheme, oneDarkTheme, gruvboxTheme, highContrastTheme, | |
| NAMED_THEMES, getNamedTheme, | |
| } from './named-themes.js'; |
🤖 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 `@packages/tss/src/index.ts` around lines 26 - 30, The package export list is
missing gruvboxTheme causing root imports to fail; update the re-export in
index.ts to include gruvboxTheme from './named-themes.js' alongside the other
symbols (e.g., draculaTheme, NAMED_THEMES, getNamedTheme) so the documented
public API matches the actual exports.
Karanjot786
force-pushed
the
add-mouse-click-tests
branch
from
c6f65a5 to
db56d47
Compare
Karanjot786
added
gssoc:approved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/tss/src/named-themes.ts`:
- Around line 72-83: Update the package metadata and tests to reflect the two
new themes: change the package description/count in packages/tss/package.json
from “eight built-in themes” to “ten built-in themes” (or equivalent wording) so
it matches README, and add test assertions in
packages/tss/src/named-themes.test.ts that reference the new exported symbols
solarizedLightTheme and tokyoNightTheme (or their exact export names) to ensure
the named-themes test suite checks for those themes' presence and expected
tokens.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 06f23fdc-77d0-461a-92a0-271471774a38
📒 Files selected for processing (3)
packages/testing/src/render.tspackages/tss/src/named-themes.tspackages/tss/src/themes.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/tss/src/themes.ts
- packages/testing/src/render.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🤖 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 `@packages/tss/src/named-themes.ts`:
- Around line 72-83: Update the package metadata and tests to reflect the two
new themes: change the package description/count in packages/tss/package.json
from “eight built-in themes” to “ten built-in themes” (or equivalent wording) so
it matches README, and add test assertions in
packages/tss/src/named-themes.test.ts that reference the new exported symbols
solarizedLightTheme and tokyoNightTheme (or their exact export names) to ensure
the named-themes test suite checks for those themes' presence and expected
tokens.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 06f23fdc-77d0-461a-92a0-271471774a38
📒 Files selected for processing (3)
packages/testing/src/render.tspackages/tss/src/named-themes.tspackages/tss/src/themes.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/tss/src/themes.ts
- packages/testing/src/render.ts
🛑 Comments failed to post (1)
packages/tss/src/named-themes.ts (1)
72-83:
⚠️ Potential issue | 🟠 Major🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Verify package.json and README.md mention the new solarizedLight theme echo "=== Checking package.json for theme count or solarizedLight reference ===" fd -t f '^package\.json$' packages/tss --exec cat {} echo "" echo "=== Checking README.md for theme count or solarizedLight reference ===" fd -t f '^README\.md$' packages/tss --exec cat {} echo "" echo "=== Checking test coverage for solarizedLight and tokyoNight ===" rg -n "solarizedLight|tokyoNight" packages/tss/src/named-themes.test.tsRepository: Karanjot786/TermUI
Length of output: 4674
Fix theme-count metadata and align tests with new built-in themes
packages/tss/package.jsonstill describes the package as having “eight built-in themes”, whilepackages/tss/README.mdlists 10 themes includingsolarizedLightandtokyo-night; update the package.json theme count/description to match.- Ensure the layer/PR description and named-themes test coverage include the newly added
solarizedLightTheme(searchingpackages/tss/src/named-themes.test.tsshows nosolarizedLight/tokyoNightmatches).🤖 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 `@packages/tss/src/named-themes.ts` around lines 72 - 83, Update the package metadata and tests to reflect the two new themes: change the package description/count in packages/tss/package.json from “eight built-in themes” to “ten built-in themes” (or equivalent wording) so it matches README, and add test assertions in packages/tss/src/named-themes.test.ts that reference the new exported symbols solarizedLightTheme and tokyoNightTheme (or their exact export names) to ensure the named-themes test suite checks for those themes' presence and expected tokens.
Contributor
|
hey u have submitted the pr for wrong issue #502 |
Karanjot786
force-pushed
the
add-mouse-click-tests
branch
from
db56d47 to
a0c1617
Compare
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
3 participants
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.
Description
Implemented
fireMouseandclickhelpers in theTestInstancereturned byrender()to simulate mouse interactions at terminal cell coordinates.Changes Made
fireMouse(x, y)helper for mouse event simulationclick(x, y)helper for full click interaction testingRelated Issue
Closes #503
Summary by CodeRabbit