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

Support lineHeight in DOM renderer #1700

Closed
pelmers opened this issue Sep 18, 2018 · 5 comments
Closed

Support lineHeight in DOM renderer #1700

pelmers opened this issue Sep 18, 2018 · 5 comments
Labels
good first issue help wanted type/enhancement Features or improvements to existing features

Comments

@pelmers
Copy link

pelmers commented Sep 18, 2018

The line height configuration is only supported in the canvas renderer (as of 3.6.0).
pointer:

this._rowContainer.style.lineHeight = 'normal';

I thought I had seen an issue for this already but wasn't able to find it now.

@Tyriar
Copy link
Member

Tyriar commented Sep 18, 2018

I don't think there's an issue but there's a comment calling this out.

* - Line height

Open to PRs.

@Tyriar Tyriar added type/enhancement Features or improvements to existing features area/renderer help wanted good first issue labels Sep 18, 2018
@Tyriar
Copy link
Member

Tyriar commented Sep 18, 2018

You can see how this used to be handled by looking at the old DOM renderer in #938

@leomoty
Copy link
Contributor

leomoty commented Oct 7, 2018

Hello @Tyriar, I would like to tackle this one, is it just a matter of using lineHeight from options inside DomRenderer.ts and defaulting to normal otherwise?

@Tyriar
Copy link
Member

Tyriar commented Oct 7, 2018

@leomoty great!

We want it to act like line height in the canvas renderer, which is used here:

// Calculate the scaled cell height, if lineHeight is not 1 then the value
// will be floored because since lineHeight can never be lower then 1, there
// is a guarentee that the scaled line height will always be larger than
// scaled char height.
this.dimensions.scaledCellHeight = Math.floor(this.dimensions.scaledCharHeight * this._terminal.options.lineHeight);
// Calculate the y coordinate within a cell that text should draw from in
// order to draw in the center of a cell.
this.dimensions.scaledCharTop = this._terminal.options.lineHeight === 1 ? 0 : Math.round((this.dimensions.scaledCellHeight - this.dimensions.scaledCharHeight) / 2);

Also the value is always set so you don't need to worry about using normal:

lineHeight: 1.0,

You can see how we used to do it by looking at src/Renderer.ts in this PR:

#938

@leomoty
Copy link
Contributor

leomoty commented Oct 29, 2018

@Tyriar this can be closed :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue help wanted type/enhancement Features or improvements to existing features
Projects
None yet
Development

No branches or pull requests

3 participants