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

Fit should use the full size instead of rounding down #389

Closed
arcanis opened this issue Dec 4, 2016 · 4 comments
Closed

Fit should use the full size instead of rounding down #389

arcanis opened this issue Dec 4, 2016 · 4 comments
Assignees

Comments

@arcanis
Copy link
Contributor

arcanis commented Dec 4, 2016

Let's say that a character height is 18px. If the terminal container has a size of 24px, then the fit algorithm will give the terminal a size of 18px, without instructing it to fill the remaining 6px in any way. It causes a kind of graphical glitch where elements looks unaligned:

image

A workaround is possible by setting height:100% on the .terminal selector, but it doesn't affect the scrollbar and, as such, doesn't resolve entirely the issue (the scrollbar still isn't aligned to the bottom of the container).

I think an acceptable fix would be to add a "pad" option on the terminal, that would instruct the terminal to add N pixels at the top of its elements (both inside the scrollable content and on the scrollbar height itself). The fit algorithm could then use this option to automatically compute the number of missing pixels, in such a way that users wouldn't have to set it themselves.

@Tyriar
Copy link
Member

Tyriar commented Dec 4, 2016

Personally I think this should be left up to the consumer to handle, it's as easy as using flex box to align to the bottom or rolling your own js to add the pad if you can't. @parisk what are your thoughts?

@arcanis
Copy link
Contributor Author

arcanis commented Dec 5, 2016

I think the main issue is that in order to support extending the scrollbar area, any userland code would have to make assumption about the html elements generated by xterm.js, assumptions that might break at any time (for example I believe you recently reworked the scrollbar). I would feel more confident if the library was handling this instead of me. At the very least, even if it's not in the library core itself, the fit addon should probably take care of this and add the required styles all by itself.

@Tyriar
Copy link
Member

Tyriar commented Dec 5, 2016

Now that you mention it there is something is a bit off with the scrollbar when the element with the way I'm positioning the terminal as the scrollbar isn't being re-positioned as well. I'll see if I can look at a way to do this nicely for the next release (time permitting).

@Tyriar
Copy link
Member

Tyriar commented Jun 1, 2018

The sizing has changed a lot since this was made, not sure it's an issue anymore

@Tyriar Tyriar closed this as completed Jun 1, 2018
@Tyriar Tyriar added the stale label Jun 1, 2018
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

2 participants