Fix text linePositionY

[andrej-ilic]

Description of change

Fixes #7344

This way the text lines are not aligned to the top. The result looks similar to the HTML example and it does not seem to break when lineHeight is set to 0.

Pre-Merge Checklist

• Documentation is changed or added
• Lint process passed (npm run lint)
• Tests passed (npm run test)

[bigtimebuddy]

Could you please remove the package-lock changes?

[andrej-ilic]

Done.

[bigtimebuddy]

Thanks @andrej-ilic, this will take a bit to verify.

[codecov]

Codecov Report

Merging #7386 (674a414) into dev (323127b) will not change coverage.
The diff coverage is n/a.

❗ Current head 674a414 differs from pull request most recent head 977d770. Consider uploading reports for the commit 977d770 to get more accurate results

@@            Coverage Diff            @@
##               dev     #7386   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           17        17           
  Lines          699       699           
=========================================
  Hits           699       699           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 323127b...977d770. Read the comment docs.

[bigtimebuddy]

This works for me, I didn't see anything unexpected fiddling with the lineHeight.

Broken: (dev)
https://jsfiddle.net/bigtimebuddy/t10wnyvf/

Fixed: (this PR)
https://jsfiddle.net/bigtimebuddy/7ygvL69p/

Build:
https://pixijs.download/dev-pr-7386/pixi.min.js

[bigtimebuddy]

@themoonrat could you let us know what you think about this Text change?

[AShenawy]

My humble 2 cents for what they're worth.
I understand that this is an issue because it's not like how HTML handles line height. But as per typography rules and definitions (see here):

Line height, also known as leading, controls the amount of space between baselines in a block of text. A text’s line height is proportional to its type size.

This means that the current way Pixi handles it is the proper way as far as typography is concerned. Having line height as a method to create a box that places text in its vertical centre isn't what line height is intended for, and can be confusing to UI designers.

[bigtimebuddy]

I agree @AShenawy that vertically centering text using line-height isn't what it's intended for, however, currently with Pixi Text objects there's no way for devs to know where the first baseline is located. It's basically a guess. That's not the case for HTML text and knowing this information can be pretty important for doing layout. This doesn't change the overall approach for line-height except for the first line.

Here's an example that illustrates the issue. Look add the line and how it stays relative to the text baseline.

This PR
https://jsfiddle.net/bigtimebuddy/qp7ejrb6/

Current Behavior
https://jsfiddle.net/bigtimebuddy/4wvoje13/

[themoonrat]

Messaged @bigtimebuddy on Slack with my concerns over this; that it will effect lots of already made content out there. It's a visually breaking change.

Also an issue of text getting cut off with lower value line heights which didn't occur before https://jsfiddle.net/e8r21yqg/ to https://jsfiddle.net/fkjvqp3m/

[andrej-ilic]

Yes, I agree. It may be bad to affect so many projects.
The last commit should fix the text getting cut off issue, in case you want to merge this PR.

[bigtimebuddy]

One easy option to have this be non-breaking is introduce a static flag for the new behavior to make it be opt-in, merge in now, and switch in v7 to this default behavior. I'm mindful not to break folks.

/**
 * New behavior for `lineHeight` that's meant to mimic HTML text. A value of `true` will
 * make sure the first baseline is offset by the `lineHeight` value if it is greater than `fontSize`. 
 * A value of `false` will use the legacy behavior and not change the baseline of the first line.
 * In the next major release, we'll enable this by default.
 * @default false
 */
Text.nextLineHeightBehavior = true

[andrej-ilic]

I made it opt-in.

I guess this changes the docs so I added the 'Documentation is changed or added' check to the PR. I'm not sure if I am supposed to check it or someone else.

[bigtimebuddy]

@nuthinking, is this good for you?

[bigtimebuddy]

Updated example with switch. Seems to work great:
https://jsfiddle.net/bigtimebuddy/7ygvL69p/

Documentation preview here:
https://pixijs.download/dev-pr-7386/docs/PIXI.Text.html#nextLineHeightBehavior

[nuthinking]

@bigtimebuddy yes, looks great. Thanks! 🙌