Skip to content

[Web] Fix terminal resizing#31576

Merged
rudream merged 1 commit intomasterfrom
yassine/fix-terminal-resizing
Sep 7, 2023
Merged

[Web] Fix terminal resizing#31576
rudream merged 1 commit intomasterfrom
yassine/fix-terminal-resizing

Conversation

@rudream
Copy link
Copy Markdown
Contributor

@rudream rudream commented Sep 7, 2023

Purpose

This PR resolves #31565

Fixes resizing not working properly in the web terminal. This was caused because we were trying to unmarshal the payload which contains number values into a map[string]string.

Demo

Before

Screen.Recording.2023-09-07.at.10.46.07.AM.mov

After

Screen.Recording.2023-09-07.at.10.50.13.AM.mov

Comment thread lib/web/terminal.go Outdated
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we check the type assertion just so this doesn't panic if another bug slips through in the future?

Also, I don't suggest changing thiw now, but we probably don't need UnmarshalTerminalParams at all - the frontend is sending

{
  "event":  "resize",
  "width":  105,
  "height": 53,
  "size":   "105:53"
}

We're taking "105:53" and parsing it to integers 105 and 53. This isn't really necessary since the UI already sent us those values as numbers. 🤷‍♂️

@rudream rudream force-pushed the yassine/fix-terminal-resizing branch from 9612728 to fed739c Compare September 7, 2023 15:53
Comment thread lib/web/terminal.go Outdated
Comment thread lib/web/terminal.go Outdated
@rudream rudream requested a review from zmb3 September 7, 2023 15:55
@rudream rudream force-pushed the yassine/fix-terminal-resizing branch from b46dfea to 94d2fd6 Compare September 7, 2023 16:01
@rudream rudream enabled auto-merge September 7, 2023 16:01
@rudream rudream added this pull request to the merge queue Sep 7, 2023
Merged via the queue into master with commit 5b01122 Sep 7, 2023
@rudream rudream deleted the yassine/fix-terminal-resizing branch September 7, 2023 16:56
@public-teleport-github-review-bot
Copy link
Copy Markdown

@rudream See the table below for backport results.

Branch Result
branch/v13 Create PR
branch/v14 Create PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Resizing web terminal is broken

3 participants