Skip to content
This repository has been archived by the owner on Mar 13, 2022. It is now read-only.

feat: render terminals in the bottom dock #14

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

akonwi
Copy link

@akonwi akonwi commented Nov 21, 2019

@the-j0k3r If we want to support moving the terminals to other docks and the center of the workspace, it's the resize logic that needs to be updated and I'm not sure where to start with that.

@@ -506,6 +506,7 @@ class StatusBar extends View {

const fromIndex = parseInt(dataTransfer.getData('from-index'))
const view = this.terminalViews[fromIndex]
if (!view) return
Copy link
Author

Choose a reason for hiding this comment

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

even with the terminals in the bottom panel, when moved, I see an error notification because view is undefined here

This comment was marked as resolved.

Copy link
Member

@the-j0k3r the-j0k3r Nov 21, 2019

Choose a reason for hiding this comment

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

Moving dock NOT panel theres an error.

[Enter steps to reproduce:]

generic

Atom: 1.41.0 x64
Electron: 4.2.7
OS: Ubuntu 18.04.3
Thrown From: terminus package 4.0.2

Stack Trace

Uncaught TypeError: Cannot read property 'destroy' of undefined

At /home/terminus/Documents/Example/terminus/lib/view.js:750

TypeError: Cannot read property 'destroy' of undefined
    at TerminusView.toggleTabView (/home/terminus/Documents/Example/terminus/lib/view.js:750:18)
    at StatusBar.onDropTabBar (/home/terminus/Documents/Example/terminus/lib/status-bar.js:514:10)
    at HTMLUListElement.tabBar.on.event (/home/terminus/Documents/Example/terminus/lib/status-bar.js:183:39)
    at HTMLUListElement.dispatch (/home/terminus/Downloads/terminus/node_modules/jquery/dist/jquery.js:4435:9)
    at HTMLUListElement.elemData.handle (/home/terminus/Downloads/terminus/node_modules/jquery/dist/jquery.js:4121:28)

Commands

Non-Core Packages

terminus 4.0.2 

FYI this works without being on dock

generic

Without this PR its actually possible to move the terminal into the bottom dock even if docks arent implemented

Copy link
Member

Choose a reason for hiding this comment

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

It looks like the toolbar on windows hides once the terminal is too big.
terminus1

Copy link
Author

Choose a reason for hiding this comment

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

Yea the input button should be in the status bar while using the dock. I hadn't gotten to that yet.

I think we should move forward with the strategy in #15

Copy link
Member

Choose a reason for hiding this comment

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

That sounds good. =)

@the-j0k3r the-j0k3r changed the title Docks feat: render terminals in the bottom dock Nov 21, 2019
@the-j0k3r the-j0k3r added the enhancement ⚙️ New feature or request label Nov 21, 2019
@@ -397,8 +397,7 @@ class StatusBar extends View {
for (let i = this.terminalViews.length - 1; i >= 0; i--) {
const view = this.terminalViews[i]
if (view) {
view.ptyProcess.terminate()
view.terminal.destroy()
view.destroy()
Copy link
Member

@the-j0k3r the-j0k3r Nov 21, 2019

Choose a reason for hiding this comment

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

Neat, does this indeed also terminate the associated ptyProcess?

})
// this.terminal.on('title', title => {
// this.title = title
// })
Copy link
Member

Choose a reason for hiding this comment

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

I'll be interested to check these title changes out. =)

Copy link
Member

@the-j0k3r the-j0k3r Nov 21, 2019

Choose a reason for hiding this comment

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

Good NewsI These changes you made into title actually fix #12 feel fee to submit them separately along with rename changes =)

Copy link
Member

Choose a reason for hiding this comment

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

Actually only fixed Linux side, Windows now doesnt even show any other name but xterm-256color

@the-j0k3r
Copy link
Member

the-j0k3r commented Nov 21, 2019

I see another thing here, the keyboard icon wasnt moved to the statusbar so there's actual more usable space.

Capture

The original coffescript PR iirc this was moved to statusbar , but its strange you can add the title to the docks but cant fit the keyboard in there too?

EDIT: I dont get it. Linux/macOS looks like above scrrenshot and Windows doesnt have the keyboard icon anywhere while in dock mode.

Capture

@UziTech
Copy link
Member

UziTech commented Nov 21, 2019

Since there is already a way to have a terminal in the bottom dock (open a terminal and drag it to the bottom dock) why not just do that by default when the config checkbox is checked

I implemented a POC of that in #15

@akonwi
Copy link
Author

akonwi commented Nov 21, 2019

@UziTech Great idea. When I first created the pr in the platformio repo, I had no idea that was possible

@UziTech
Copy link
Member

UziTech commented Nov 22, 2019

@akonwi it would probably be easier to update this PR to use onDropTabBar code to move it to the dock then use #15

@the-j0k3r the-j0k3r added the W.I.P. 🚧 In progress and not ready for merging label Feb 27, 2020
@UziTech UziTech mentioned this pull request Mar 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement ⚙️ New feature or request W.I.P. 🚧 In progress and not ready for merging
Development

Successfully merging this pull request may close these issues.

3 participants