Skip to content

Conversation

@notAreYouScared
Copy link
Member

@notAreYouScared notAreYouScared commented Sep 28, 2025

Fixes: #1752
Before the last character would be hidden behind the scrollbar.
Before:
image

After:
image

Appears to fix 1752
@notAreYouScared notAreYouScared added this to the beta26 milestone Sep 28, 2025
@notAreYouScared notAreYouScared self-assigned this Sep 28, 2025
@coderabbitai
Copy link

coderabbitai bot commented Sep 28, 2025

📝 Walkthrough

Walkthrough

Added @xterm/addon-canvas dependency, exposed CanvasAddon on window.Xterm, and integrated CanvasAddon into the server console terminal by instantiating and loading it during terminal setup. Minor formatting change in clipboard call.

Changes

Cohort / File(s) Summary
Dependencies
package.json
Added dependency @xterm/addon-canvas at version ^0.7.0.
Xterm Globals
resources/js/console.js
Imported CanvasAddon from @xterm/addon-canvas and attached it to window.Xterm.
Server Console Integration
resources/views/filament/components/server-console.blade.php
Imported and instantiated CanvasAddon; loaded it into the Terminal instance; minor semicolon formatting in clipboard writeText.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor User
    participant Page as Server Console View
    participant Xterm as Terminal (xterm.js)
    participant Canvas as CanvasAddon (@xterm/addon-canvas)
    participant Global as window.Xterm

    Note over Global: console.js attaches CanvasAddon to window.Xterm

    User->>Page: Open server console page
    Page->>Xterm: new Terminal(...)
    Page->>Canvas: new CanvasAddon()
    Page->>Xterm: terminal.loadAddon(CanvasAddon)
    Note right of Xterm: Terminal renders using Canvas addon
Loading

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title “Add xtermjs Canvas” succinctly identifies the core change of integrating the xterm.js Canvas addon, which is the central modification in this pull request. It concisely conveys the key addition without extraneous details and aligns with the implementation of CanvasAddon in the code. While it does not explicitly mention the scrollbar fix, it accurately reflects the new functionality that enables the rendering improvement.
Description Check ✅ Passed The pull request description directly relates to the changes by referencing issue #1752, describing the problem of the last character being hidden behind the scrollbar, and providing before-and-after screenshots that clearly demonstrate the effect of the new canvas-based rendering. It stays on topic and explains the intent and outcome of the PR without deviating into unrelated details.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9f35f1c and 7febb65.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (3)
  • package.json (1 hunks)
  • resources/js/console.js (1 hunks)
  • resources/views/filament/components/server-console.blade.php (2 hunks)
🔇 Additional comments (3)
package.json (1)

20-20: Dependency addition LGTM.

@xterm/addon-canvas at ^0.7.0 lines up with the existing @xterm/xterm ^5.5.0 stack, so the dependency wiring looks correct.

resources/js/console.js (1)

6-11: Global exposure looks good.

Adding CanvasAddon to the window.Xterm bundle keeps the Blade-side destructuring consistent with the other addons.

resources/views/filament/components/server-console.blade.php (1)

79-92: Renderer hookup LGTM.

Instantiating CanvasAddon alongside the existing fit/search/web-links addons and loading it before terminal.open(...) follows xterm’s expected pattern, so the canvas renderer should activate reliably.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@notAreYouScared notAreYouScared merged commit ec5fd32 into main Sep 28, 2025
25 checks passed
@notAreYouScared notAreYouScared deleted the charles/1752 branch September 28, 2025 19:17
@github-actions github-actions bot locked and limited conversation to collaborators Sep 28, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Too long messages in console go off "screen"

3 participants