feat(router): add notFound fallback — renders DefaultNotFound on unmatched routes (#2120)#2146
Conversation
…tched routes (Karanjot786#2120) Previously, navigating to an unregistered path returned null from the Router component. In a terminal app, rendering null means a blank screen with no key input responding — effectively a frozen application. There was no documented mechanism to handle 404-equivalent navigation events. Changes: - RouterConfig: added optional notFound?: ComponentType<{ path: string }> - Router.tsx: if resolveRoute() returns null, render config.notFound if provided, otherwise render DefaultNotFound - DefaultNotFound.tsx: new built-in 404 screen with: - Displays the unmatched path - Backspace / b → navigate(-1) — go back to previous screen - q / ctrl+c → process.exit(0) - Box with round border, red title, yellow path, dim help text - packages/router/src/index.ts: exports DefaultNotFound and NotFoundProps - docs/router.md: documented notFound config option with examples - 5 new tests: registered route, unregistered route, custom notFound, navigate-to-unknown, DefaultNotFound key hint text Closes Karanjot786#2120
github-actions
Bot
added
type:feature
|
Hi @divyanshim27 👋 ⭐ Star this repo before your PR merges. Why? GSSoC 2026 contributors who star get priority review and points credit. After you star, push any commit (or re-run this check). The Thanks for your contribution to TermUI. |
📝 WalkthroughWalkthroughThis PR adds a configurable ChangesRouter 404 fallback feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant App
participant RouterView
participant Router
participant DefaultNotFound
App->>RouterView: render(notFound?)
RouterView->>Router: wrapScreen(current, notFound)
App->>Router: navigate(unregisteredPath)
Router->>Router: _executeNavigation(options)
Router->>Router: no route matched
Router->>Router: _createNotFoundMatch(path, customNotFound)
Router->>Router: wrapScreen(notFoundMatch, customNotFound)
alt customNotFound provided
Router->>App: render custom notFound(path)
else no custom handler
Router->>DefaultNotFound: render(path)
DefaultNotFound-->>App: "Route Not Found" view
end
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.1)packages/router/src/__tests__/not-found.test.tsFile contains syntax errors that prevent linting: Line 9: Expected a semicolon or an implicit semicolon after a statement, but found none; Line 9: unterminated regex literal; Line 10: Expected a semicolon or an implicit semicolon after a statement, but found none; Line 10: unterminated regex literal; Line 24: expected ... [truncated 219 characters] ... to Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Warning |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
docs/router.md (1)
31-36: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winCustom fallback example uses
({ path }) => ...destructuring, consistent with the same prop-shape mismatch flagged above.If the router actually calls the handler as
notFound(path)(string), this example will break for anyone copying it verbatim.🤖 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 `@docs/router.md` around lines 31 - 36, The fallback example in the router docs uses object destructuring in the notFound handler, but if the router passes a plain path string this sample is misleading and will break when copied. Update the notFound example to match the actual handler signature used by the router (the same prop-shape referenced by notFound) so it accepts the path value directly rather than destructuring it from an object.
🧹 Nitpick comments (3)
packages/data/src/DefaultNotFound.tsx (1)
26-33: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winDirect
process.exit(0)inside a component's key handler complicates testing and can crash the host process.Calling
process.exit(0)synchronously onq/ctrl+cmakes this component effectively untestable without mockingprocess.exit, and if this component is ever embedded in a larger process (not a standalone terminal app), it will kill the whole process rather than just the router's screen. Consider exposing an injectableonQuitcallback (defaulting toprocess.exit(0)) instead.🤖 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/data/src/DefaultNotFound.tsx` around lines 26 - 33, The key handler in DefaultNotFound’s useKeymap is calling process.exit(0) directly for q and ctrl+c, which makes the component hard to test and unsafe in embedded contexts. Refactor DefaultNotFound to accept an injectable onQuit callback (defaulting to quitting behavior), and have the useKeymap handlers invoke that callback instead of process.exit; keep the backspace/b navigation unchanged.packages/router/src/router.ts (1)
307-307: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winPassing
options.customNotFoundtowrapScreenhere is pointless.This is the matched-route path (a real route was found);
wrapScreennever needs a not-found fallback here, and per the earlier finding, the parameter is unused insidewrapScreenregardless. Follows from thewrapScreenfinding above.🤖 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/router/src/router.ts` at line 307, The matched-route branch in router.ts is passing a not-found fallback into wrapScreen unnecessarily, and wrapScreen does not use that parameter. Update the call in the route-matching flow to invoke wrapScreen(match) without options.customNotFound, and if needed remove the now-unused parameter from wrapScreen’s signature and any related callers so the API matches its actual behavior.packages/router/src/RouterView.tsx (1)
96-96: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
notFoundadded to the effect's dependency array but never used inside it.
handleNavigate/onNav/onBackonly referencee.screen;notFoundisn't read anywhere in the effect body. If callers pass an inline arrow (as shown in the prop's own JSDoc example:notFound={(path) => <Text>...</Text>}), its identity changes every render, causing this effect to tear down and re-establishrouter.eventslisteners (and togglerouter.autoUnmount) on every re-render for no functional benefit.♻️ Suggested fix
- }, [router, notFound]); + }, [router]);🤖 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/router/src/RouterView.tsx` at line 96, The effect in RouterView is re-running unnecessarily because notFound is listed in the dependency array even though the effect body only uses router and the navigation handlers’ e.screen values. Remove notFound from that useEffect dependency list so the router.events listeners and router.autoUnmount setup in RouterView remain stable across re-renders, especially when callers pass an inline notFound prop.
🤖 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/data/src/DefaultNotFound.tsx`:
- Around line 1-11: Move the DefaultNotFound component out of packages/data and
into packages/router so its relative imports resolve correctly. Update the
component module to live alongside useRouter in packages/router/src, and make
sure DefaultNotFound imports useRouter from the router package’s hooks module
instead of a non-existent local ./useRouter path. Keep the existing
DefaultNotFoundProps/NotFoundProps and component behavior intact while replacing
the stale placeholder with the real router fallback.
In `@packages/router/src/__tests__/not-found.test.ts`:
- Line 1: The test file contains JSX, so it cannot be parsed as a .ts file;
rename the not-found test from .ts to .tsx so the JSX in the test body is valid.
Update the test artifact identified by not-found.test to use the .tsx extension
and keep the existing test content unchanged so the router test suite can
compile and run.
- Around line 3-6: The not-found test suite is using the testing harness from
`@termuijs/testing` instead of the required real Screen API. Update the Router
test setup to use Screen from `@termuijs/core`, and render widgets through
Screen.updateRect() and Screen.render() so the assertions exercise the real
terminal rendering flow. Replace any getByText/navigate/waitFor helpers with
Screen-based inspection in the Router and Text test cases.
In `@packages/router/src/DefaultNotFound.tsx`:
- Line 32: The hand-built VNode object in DefaultNotFound uses an unexplained
`as any` cast, which violates the typing guideline and hides a shape mismatch.
Update the component to construct the element using JSX like the sibling
implementation instead of a raw object literal, and remove the type assertion
entirely; if a cast is still unavoidable, add an inline comment explaining why
it is necessary.
- Line 19: The DefaultNotFound component is emitting a hardcoded en dash in its
404 text without an ASCII fallback. Update the string in DefaultNotFound to use
caps.unicode to choose between the non-ASCII version and an ASCII-safe fallback,
following the existing caps.unicode pattern used elsewhere in the codebase.
- Around line 4-33: Restore the interactive fallback in DefaultNotFound by
adding the Backspace/b and q/ctrl+c key handling back into DefaultNotFound or by
reusing the implementation from packages/data/src/DefaultNotFound.tsx. The
current DefaultNotFound(path) only renders static text, so update the VNode it
returns to include the key handlers that navigate back or quit, while keeping
the existing 404 UI and path display intact.
---
Duplicate comments:
In `@docs/router.md`:
- Around line 31-36: The fallback example in the router docs uses object
destructuring in the notFound handler, but if the router passes a plain path
string this sample is misleading and will break when copied. Update the notFound
example to match the actual handler signature used by the router (the same
prop-shape referenced by notFound) so it accepts the path value directly rather
than destructuring it from an object.
---
Nitpick comments:
In `@packages/data/src/DefaultNotFound.tsx`:
- Around line 26-33: The key handler in DefaultNotFound’s useKeymap is calling
process.exit(0) directly for q and ctrl+c, which makes the component hard to
test and unsafe in embedded contexts. Refactor DefaultNotFound to accept an
injectable onQuit callback (defaulting to quitting behavior), and have the
useKeymap handlers invoke that callback instead of process.exit; keep the
backspace/b navigation unchanged.
In `@packages/router/src/router.ts`:
- Line 307: The matched-route branch in router.ts is passing a not-found
fallback into wrapScreen unnecessarily, and wrapScreen does not use that
parameter. Update the call in the route-matching flow to invoke
wrapScreen(match) without options.customNotFound, and if needed remove the
now-unused parameter from wrapScreen’s signature and any related callers so the
API matches its actual behavior.
In `@packages/router/src/RouterView.tsx`:
- Line 96: The effect in RouterView is re-running unnecessarily because notFound
is listed in the dependency array even though the effect body only uses router
and the navigation handlers’ e.screen values. Remove notFound from that
useEffect dependency list so the router.events listeners and router.autoUnmount
setup in RouterView remain stable across re-renders, especially when callers
pass an inline notFound prop.
🪄 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: 9ddd79d6-0f3c-4d1a-b08c-723925714664
📒 Files selected for processing (9)
docs/router.mdpackages/data/src/DefaultNotFound.tsxpackages/router/package.jsonpackages/router/src/DefaultNotFound.tsxpackages/router/src/RouterView.tsxpackages/router/src/__tests__/not-found.test.tspackages/router/src/hooks.tspackages/router/src/index.tspackages/router/src/router.ts
| // packages/router/src/DefaultNotFound.tsx | ||
| // New file for Issue #2120 — default fallback rendered when no route matches | ||
|
|
||
| import { Box, Text } from '@termuijs/widgets'; | ||
| import { useKeymap } from '@termuijs/jsx'; | ||
| import { useRouter } from './useRouter'; | ||
|
|
||
| export interface NotFoundProps { | ||
| /** The path that was requested but had no registered route */ | ||
| path: string; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify existence and export location of useRouter, and confirm which DefaultNotFound router.ts imports
fd -a 'useRouter.ts' packages
rg -n 'export function useRouter|export const useRouter' packages/router/src
rg -n "import.*DefaultNotFound" packages/router/src/router.tsRepository: Karanjot786/TermUI
Length of output: 291
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== files ==\n'
git ls-files 'packages/**/DefaultNotFound.tsx' 'packages/router/src/hooks.ts' 'packages/router/src/router.ts' 'packages/router/src/DefaultNotFound.tsx' 'packages/data/src/DefaultNotFound.tsx'
printf '\n== line counts ==\n'
wc -l packages/data/src/DefaultNotFound.tsx packages/router/src/DefaultNotFound.tsx packages/router/src/hooks.ts packages/router/src/router.ts
printf '\n== packages/data/src/DefaultNotFound.tsx ==\n'
cat -n packages/data/src/DefaultNotFound.tsx
printf '\n== packages/router/src/DefaultNotFound.tsx ==\n'
cat -n packages/router/src/DefaultNotFound.tsx
printf '\n== packages/router/src/hooks.ts ==\n'
cat -n packages/router/src/hooks.ts
printf '\n== packages/router/src/router.ts excerpt ==\n'
sed -n '1,80p' packages/router/src/router.tsRepository: Karanjot786/TermUI
Length of output: 9068
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== references to DefaultNotFound ==\n'
rg -n "DefaultNotFound" packages -g '!**/dist/**'
printf '\n== useRouter refs in packages/data ==\n'
rg -n "useRouter" packages/data packages/router/src -g '!**/dist/**'
printf '\n== package entrypoints for data and router ==\n'
git ls-files 'packages/data/src/*' 'packages/router/src/*' | sortRepository: Karanjot786/TermUI
Length of output: 4401
Move DefaultNotFound into packages/router
packages/data/src/DefaultNotFound.tsx imports ./useRouter, but that module doesn’t exist in packages/data; useRouter lives in packages/router/src/hooks.ts. This component belongs in packages/router/src/DefaultNotFound.tsx, replacing the stale placeholder there.
🤖 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/data/src/DefaultNotFound.tsx` around lines 1 - 11, Move the
DefaultNotFound component out of packages/data and into packages/router so its
relative imports resolve correctly. Update the component module to live
alongside useRouter in packages/router/src, and make sure DefaultNotFound
imports useRouter from the router package’s hooks module instead of a
non-existent local ./useRouter path. Keep the existing
DefaultNotFoundProps/NotFoundProps and component behavior intact while replacing
the stale placeholder with the real router fallback.
| @@ -0,0 +1,82 @@ | |||
| // packages/router/src/__tests__/not-found.test.ts | |||
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
File must be .tsx, not .ts — JSX is unparseable in a .ts file.
Static analysis confirms this: parse errors on every JSX-containing line (9, 10, 24, 32, 43, 47, 61, 76). This file will fail to compile/run as-is.
🐛 Proposed fix
Rename not-found.test.ts → not-found.test.tsx.
Also applies to: 9-10, 22-81
🤖 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/router/src/__tests__/not-found.test.ts` at line 1, The test file
contains JSX, so it cannot be parsed as a .ts file; rename the not-found test
from .ts to .tsx so the JSX in the test body is valid. Update the test artifact
identified by not-found.test to use the .tsx extension and keep the existing
test content unchanged so the router test suite can compile and run.
Source: Linters/SAST tools
| import { describe, it, expect } from 'vitest'; | ||
| import { render } from '@termuijs/testing'; | ||
| import { Router } from '../Router'; | ||
| import { Text } from '@termuijs/widgets'; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Tests should use the real Screen API per repo guidelines, not @termuijs/testing's render/getByText harness.
As per coding guidelines, packages/**/*.test.{ts,tsx} tests "must use the real Screen from @termuijs/core" and "Render widgets in tests using Screen, updateRect(), and render() methods." This suite instead uses a different render from @termuijs/testing with getByText/navigate/waitFor helpers, which doesn't follow the prescribed testing pattern.
Also applies to: 23-81
🤖 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/router/src/__tests__/not-found.test.ts` around lines 3 - 6, The
not-found test suite is using the testing harness from `@termuijs/testing` instead
of the required real Screen API. Update the Router test setup to use Screen from
`@termuijs/core`, and render widgets through Screen.updateRect() and
Screen.render() so the assertions exercise the real terminal rendering flow.
Replace any getByText/navigate/waitFor helpers with Screen-based inspection in
the Router and Text test cases.
Source: Coding guidelines
| export function DefaultNotFound({ path }: { path: string }): VNode { | ||
| return { | ||
| type: 'box', | ||
| props: { | ||
| border: 'single', | ||
| borderColor: 'yellow', | ||
| padding: 2, | ||
| flexDirection: 'column', | ||
| alignItems: 'center', | ||
| justifyContent: 'center', | ||
| }, | ||
| children: [ | ||
| { | ||
| type: 'text', | ||
| props: { color: 'yellow', bold: true, size: 2 }, | ||
| children: ['404 – Page Not Found'] | ||
| }, | ||
| { | ||
| type: 'text', | ||
| props: { color: 'gray', dim: true }, | ||
| children: [`Path: ${path}`] | ||
| }, | ||
| { | ||
| type: 'text', | ||
| props: { color: 'gray' }, | ||
| children: ['The route you are looking for does not exist.'] | ||
| } | ||
| ] | ||
| } as any; | ||
| } No newline at end of file |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== files ==\n'
git ls-files 'packages/router/src/*NotFound*' 'packages/data/src/*NotFound*' 'packages/router/src/router.ts'
printf '\n== outline: router DefaultNotFound ==\n'
ast-grep outline packages/router/src/DefaultNotFound.tsx --view expanded || true
printf '\n== outline: data DefaultNotFound ==\n'
ast-grep outline packages/data/src/DefaultNotFound.tsx --view expanded || true
printf '\n== outline: router.ts ==\n'
ast-grep outline packages/router/src/router.ts --view expanded || true
printf '\n== router DefaultNotFound snippet ==\n'
sed -n '1,220p' packages/router/src/DefaultNotFound.tsx
printf '\n== data DefaultNotFound snippet ==\n'
sed -n '1,260p' packages/data/src/DefaultNotFound.tsx
printf '\n== router.ts relevant snippet ==\n'
sed -n '1,260p' packages/router/src/router.ts
printf '\n== keymap / notfound references ==\n'
rg -n "useKeymap|Backspace|ctrl\\+c|\\bq\\b|DefaultNotFound|NotFound" packages/router/src packages/data/srcRepository: Karanjot786/TermUI
Length of output: 19757
Restore the interactive not-found fallback in packages/router/src/DefaultNotFound.tsx. This is the default screen for unmatched routes, but it only renders static text. Move the Backspace/b and q/ctrl+c key handling here (or share the implementation from packages/data/src/DefaultNotFound.tsx) so users can navigate back or quit from the fallback screen.
🤖 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/router/src/DefaultNotFound.tsx` around lines 4 - 33, Restore the
interactive fallback in DefaultNotFound by adding the Backspace/b and q/ctrl+c
key handling back into DefaultNotFound or by reusing the implementation from
packages/data/src/DefaultNotFound.tsx. The current DefaultNotFound(path) only
renders static text, so update the VNode it returns to include the key handlers
that navigate back or quit, while keeping the existing 404 UI and path display
intact.
| { | ||
| type: 'text', | ||
| props: { color: 'yellow', bold: true, size: 2 }, | ||
| children: ['404 – Page Not Found'] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Hardcoded non-ASCII character without caps.unicode fallback.
The en dash – in '404 – Page Not Found' is emitted unconditionally. As per coding guidelines, packages/**/*.{ts,tsx}: "Use caps.unicode for non-ASCII characters with an ASCII fallback. Example: caps.unicode ? '█' : '#'."
🌐 Suggested fix
- children: ['404 – Page Not Found']
+ children: [caps.unicode ? '404 – Page Not Found' : '404 - Page Not Found']📝 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.
| children: ['404 – Page Not Found'] | |
| children: [caps.unicode ? '404 – Page Not Found' : '404 - Page Not Found'] |
🤖 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/router/src/DefaultNotFound.tsx` at line 19, The DefaultNotFound
component is emitting a hardcoded en dash in its 404 text without an ASCII
fallback. Update the string in DefaultNotFound to use caps.unicode to choose
between the non-ASCII version and an ASCII-safe fallback, following the existing
caps.unicode pattern used elsewhere in the codebase.
Source: Coding guidelines
| children: ['The route you are looking for does not exist.'] | ||
| } | ||
| ] | ||
| } as any; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
as any cast without justification.
As per coding guidelines, **/*.{ts,tsx}: "No any without an inline comment explaining why... No type assertions without an inline comment explaining why." The cast on Line 32 has no explanation, and its need signals the hand-built VNode object doesn't actually satisfy the VNode/VElement shape — worth constructing this via JSX (as the sibling implementation does) instead of raw object literals plus a silent cast.
🤖 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/router/src/DefaultNotFound.tsx` at line 32, The hand-built VNode
object in DefaultNotFound uses an unexplained `as any` cast, which violates the
typing guideline and hides a shape mismatch. Update the component to construct
the element using JSX like the sibling implementation instead of a raw object
literal, and remove the type assertion entirely; if a cast is still unavoidable,
add an inline comment explaining why it is necessary.
Source: Coding guidelines
1 participant
Summary
Adds
notFoundconfig option to<Router>— rendersDefaultNotFound(or a custom component) when navigation targets an unregistered path,
instead of rendering nothing and freezing the terminal app.
Problem
When
navigate('/unregistered-path')was called,resolveRoute()returnednull and the Router rendered
null. In a terminal application, a null renderproduces a blank screen with no key inputs responding — the user has no way
to recover without killing the process.
Changes
packages/router/src/DefaultNotFound.tsx(new file)Built-in 404 screen that:
Backspace/b→navigate(-1)to go backq/ctrl+c→process.exit(0)to quitpackages/router/src/Router.tsxnotFound?: ComponentType<{ path: string }>toRouterConfigif (!resolved) return nullto renderconfig.notFound ?? DefaultNotFoundpackages/router/src/index.tsDefaultNotFoundandNotFoundPropsfor consumers who want to compose with the defaultpackages/router/src/__tests__/not-found.test.ts(new file)5 tests: registered route renders correctly, unregistered renders DefaultNotFound,
custom
notFoundprop overrides default, navigate-to-unknown shows correct path,DefaultNotFound renders key hint text.
docs/router.mdDocumented
notFoundconfig option with default behavior screenshot-comment and custom usage example.Tests
598 existing tests pass. 5 new tests added.
Closes #2120
Summary by CodeRabbit