search

fugazi-io / webclient

web based terminal application for executing local and remote commands
http://fugazi.io
28 stars 12 forks source link

automatic scroll down breaks sometimes #20

Open nitzantomer opened 7 years ago

nitzantomer commented 7 years ago

not quite sure when, find a way to reproduce first.

TomZelazny commented 7 years ago

This is because of the checkOverflow function in output.tsx:

private checkOverflow(): boolean {
    var node: any = ReactDOM.findDOMNode(this);
    return node.scrollTop + node.offsetHeight === node.scrollHeight;
}

it seems as though under certain conditions return node.scrollTop + node.offsetHeight === node.scrollHeight; does not correctly check if the scrollbar is at the bottom of the page. for instance screen resizes (opening the console, zooming in/out, resizing the window) or zoomed windows.

an elegant fix (which makes having to check for overflow obsolete ) is changing 

main#ui article.terminal section.output{
    ....
    flex-direction: column;
    ....

into

main#ui article.terminal section.output{
    ....
    flex-direction: column-reverse;
    ....

but it seems to break the scrollbar. If anyone has any ideas on why checkOverflow is wrong I'd be happey to hear.

nitzantomer commented 7 years ago

You're right, it breaks the scrollbar.

I flowed your advice and changed it to: Math.ceil(this.element.scrollTop + this.element.offsetHeight) >= this.element.scrollHeight I also updated the scrollbar lib version and removed a use of deprecated react feature.

The PR is here: https://github.com/fugazi-io/webclient/pull/33

TomZelazny commented 7 years ago

After testing, the Math.ceil does not fix the problem, any fix that uses scrollTop, offestHeight and scollHeight or other browser window properties seems pretty susceptible to alot of edge cases. I am currently abroad for work so I can't really test this properly but I think it would be much simpler and more elegant to use a css solution (and just fix the css for the scrollbar so it does not disappear). If you can reopen the issue I'll work on fixing it when I get back.

nitzantomer commented 7 years ago

@TomZelazny Sorry, I forgot about it... Re-opened it, if you still wanna take a look. I agree that a css only solution is the best, but it might be tricky..

nitzantomer commented 7 years ago

Scrolling related PR: https://github.com/fugazi-io/webclient/pull/58 Not sure if it fixed this bug completely