Skip to content

Commit 9f3c3ad

Browse files
dannotectate
andauthored
fix(screenshot): support refs and improve error messages (#141)
* fix(screenshot): support refs and improve error messages * fix(cli): support selector argument in screenshot command * Fix CSS class selectors being treated as file paths * fix(test): update screenshot test assertions --------- Co-authored-by: Chris Tate <chris@ctate.dev>
1 parent c046de2 commit 9f3c3ad

2 files changed

Lines changed: 83 additions & 13 deletions

File tree

cli/src/commands.rs

Lines changed: 69 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -335,11 +335,33 @@ pub fn parse_command(args: &[String], flags: &Flags) -> Result<Value, ParseError
335335

336336
// === Screenshot/PDF ===
337337
"screenshot" => {
338-
let mut cmd = json!({ "id": id, "action": "screenshot", "fullPage": flags.full });
339-
if let Some(path) = rest.get(0) {
340-
cmd["path"] = json!(path);
341-
}
342-
Ok(cmd)
338+
// screenshot [selector] [path]
339+
// selector: @ref or CSS selector
340+
// path: file path (contains / or . or ends with known extension)
341+
let (selector, path) = match (rest.get(0), rest.get(1)) {
342+
(Some(first), Some(second)) => {
343+
// Two args: first is selector, second is path
344+
(Some(*first), Some(*second))
345+
}
346+
(Some(first), None) => {
347+
// One arg: determine if it's a selector or a path
348+
let is_relative_path = first.starts_with("./") || first.starts_with("../");
349+
let is_selector = !is_relative_path
350+
&& (first.starts_with('.') || first.starts_with('#') || first.starts_with('@'));
351+
let has_path_extension = first.ends_with(".png")
352+
|| first.ends_with(".jpg")
353+
|| first.ends_with(".jpeg")
354+
|| first.ends_with(".webp");
355+
let is_path = is_relative_path || first.contains('/') || has_path_extension;
356+
if is_selector || !is_path {
357+
(Some(*first), None)
358+
} else {
359+
(None, Some(*first))
360+
}
361+
}
362+
_ => (None, None),
363+
};
364+
Ok(json!({ "id": id, "action": "screenshot", "path": path, "selector": selector, "fullPage": flags.full }))
343365
}
344366
"pdf" => {
345367
let path = rest.get(0).ok_or_else(|| ParseError::MissingArguments {
@@ -1504,7 +1526,8 @@ mod tests {
15041526
fn test_screenshot() {
15051527
let cmd = parse_command(&args("screenshot"), &default_flags()).unwrap();
15061528
assert_eq!(cmd["action"], "screenshot");
1507-
assert!(cmd.get("path").is_none());
1529+
assert_eq!(cmd["path"], serde_json::Value::Null);
1530+
assert_eq!(cmd["selector"], serde_json::Value::Null);
15081531
}
15091532

15101533
#[test]
@@ -1523,6 +1546,46 @@ mod tests {
15231546
assert_eq!(cmd["fullPage"], true);
15241547
}
15251548

1549+
#[test]
1550+
fn test_screenshot_with_ref() {
1551+
let cmd = parse_command(&args("screenshot @e1"), &default_flags()).unwrap();
1552+
assert_eq!(cmd["action"], "screenshot");
1553+
assert_eq!(cmd["selector"], "@e1");
1554+
assert_eq!(cmd["path"], serde_json::Value::Null);
1555+
}
1556+
1557+
#[test]
1558+
fn test_screenshot_with_css_class() {
1559+
let cmd = parse_command(&args("screenshot .my-button"), &default_flags()).unwrap();
1560+
assert_eq!(cmd["action"], "screenshot");
1561+
assert_eq!(cmd["selector"], ".my-button");
1562+
assert_eq!(cmd["path"], serde_json::Value::Null);
1563+
}
1564+
1565+
#[test]
1566+
fn test_screenshot_with_css_id() {
1567+
let cmd = parse_command(&args("screenshot #header"), &default_flags()).unwrap();
1568+
assert_eq!(cmd["action"], "screenshot");
1569+
assert_eq!(cmd["selector"], "#header");
1570+
assert_eq!(cmd["path"], serde_json::Value::Null);
1571+
}
1572+
1573+
#[test]
1574+
fn test_screenshot_with_path() {
1575+
let cmd = parse_command(&args("screenshot ./output.png"), &default_flags()).unwrap();
1576+
assert_eq!(cmd["action"], "screenshot");
1577+
assert_eq!(cmd["selector"], serde_json::Value::Null);
1578+
assert_eq!(cmd["path"], "./output.png");
1579+
}
1580+
1581+
#[test]
1582+
fn test_screenshot_with_selector_and_path() {
1583+
let cmd = parse_command(&args("screenshot .btn ./button.png"), &default_flags()).unwrap();
1584+
assert_eq!(cmd["action"], "screenshot");
1585+
assert_eq!(cmd["selector"], ".btn");
1586+
assert_eq!(cmd["path"], "./button.png");
1587+
}
1588+
15261589
// === Snapshot ===
15271590

15281591
#[test]

src/actions.ts

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -549,15 +549,22 @@ async function handleScreenshot(
549549

550550
let target: Page | ReturnType<Page['locator']> = page;
551551
if (command.selector) {
552-
target = page.locator(command.selector);
552+
target = browser.getLocator(command.selector);
553553
}
554554

555-
if (command.path) {
556-
await target.screenshot({ ...options, path: command.path });
557-
return successResponse(command.id, { path: command.path });
558-
} else {
559-
const buffer = await target.screenshot(options);
560-
return successResponse(command.id, { base64: buffer.toString('base64') });
555+
try {
556+
if (command.path) {
557+
await target.screenshot({ ...options, path: command.path });
558+
return successResponse(command.id, { path: command.path });
559+
} else {
560+
const buffer = await target.screenshot(options);
561+
return successResponse(command.id, { base64: buffer.toString('base64') });
562+
}
563+
} catch (error) {
564+
if (command.selector) {
565+
throw toAIFriendlyError(error, command.selector);
566+
}
567+
throw error;
561568
}
562569
}
563570

0 commit comments

Comments
 (0)