Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Double vertical scrollbar #250

Open
Privat33r-dev opened this issue May 3, 2024 · 1 comment
Open

Double vertical scrollbar #250

Privat33r-dev opened this issue May 3, 2024 · 1 comment

Comments

@Privat33r-dev
Copy link
Contributor

Privat33r-dev commented May 3, 2024

Problerm

Sometimes, double vertical scrollbar appears in rooms with multiple terminals, like https://training.play-with-docker.com/swarm-stack-intro/ . To reproduce open F12 developer console, dock it to the bottom and slowly slide its size up or down. You will see double scrollbar sometimes (between "jumps" from the script to match lines size and console px size). If you are lucky, sometimes it can be reproduced just by opening the link (I guess on my screenshot it was related to closed connection with instances).

Another issue is there is too wide gap between terminals, so the space utilization is suboptimal.

Screenshot

Solution

Change these 2 lines

margin-bottom: 15px;
overflow-y: auto;

to

margin-bottom: 5px; 
overflow: hidden;

Notes

There still is a big room for improvement:

  1. Reduce footer size/move it to the left side
  2. Implement horizontal splitter
  3. Attempt to refresh if overflow detected
@Privat33r-dev
Copy link
Contributor Author

Privat33r-dev commented May 4, 2024

Vertical scrollbar is possible and maybe it even fixes/improves things, but the term sizes are somehow synchronized and resize functions are located somewhere in the legacy code.

Anyway, it looks interesting as a concept. And is easy to implement.
image
image

To try it, open dev tools and paste this in console:

$('.term1').after($('<div class="vsplitter" style="touch-action: none; width: 100%; background: url(https://raw.githubusercontent.com/RickStrahl/jquery-resizable/master/assets/hsizegrip.png) center center no-repeat #2fa8c3; display: block; height: 15px; cursor: ns-resize;"></div>'));
$('.panel-right').css('display', 'block');

$(".term1").resizable({
    handleSelector: ".vsplitter",
    resizeWidth: false,
    grid: [0, $(".xterm-viewport").css('line-height')],
    animate: true,
    onDragEnd: pwd.resize.bind(pwd)
});

Well, after further investigation I found the reason of the issue:

  pwd.prototype.resize = function() {
    var name = Object.keys(this.instances)[0]
    for (var n in this.instances) {
      // there might be some instances without terminals
      if (this.instances[n].terms) {
        for (var i = 0; i < this.instances[n].terms.length; i ++) {
          var term = this.instances[n].terms[i];
          var size = term.proposeGeometry();
          if (size.cols && size.rows) {
            return this.socket.emit('instance viewport resize', size.cols, size.rows);
          }
        }
      }
    }
  };

somewhere in the xterm/other dependencies. It returns before completing the loop and also it doesn't provide instance for which resize is being done (first argument usually is instance*). It doesn't seem to be in the pwd sdk, so it's likely way more upstream. I decided to wrap around this experiment since I have another, better idea: separate machines on tabs.

*Example: {"name":"instance terminal in","args":["cor5c20l_cor5c28l2o9000af38f0","echo Hello!"]}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant