vadimdemedes/ink

Text wrapping behavior broken in some scenarios with yoga-web-wasm

matteodepalo opened this issue · 3 comments

Hi,
In our codebase we have some tests that check that text wrapping works as expected in boxes with a certain paddingX, and I noticed that after upgrading to 4.1.0 these tests broke. I was able to do a bisect of the ink dependency and found that this commit introduced the issue.

Here are the screenshots of before (ink 4.0.0) and after (ink 4.1.0 and 4.2.0)

Before:
Screenshot 2023-05-03 at 10 28 42

After:
Screenshot 2023-05-03 at 10 28 50

This is the code generating these

import React from 'react';
import { render, Text, Box } from 'ink';

render(
  <Box width={80} borderStyle="round" borderColor="green">
    <Box paddingX={2}>
      <Box marginLeft={2}>
        <Text>•</Text>

        <Box flexGrow={1} marginLeft={1}>
          <Text>
            Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
          </Text>
        </Box>
      </Box>
    </Box>
  </Box>
)

It seems like 4.0.0 is respecting the paddingX={2} property and wrapping the text correctly but 4.1.0+ only has one space at the end. Also, if I remove flexGrow={1} wrapping doesn't work in in 4.0.0 as well.

You can download the example repo here and flip the version from 4.0.0 to 4.2.0 to see the behavior.

Thanks for reporting this! @amitdahan Would you be able to look into this?

Hey, I'll definitely find some time to debug this, but I'm (maybe incorrectly?) assuming this is some subtle difference in the Yoga distribution we're using?

It seems yoga-wasm-web is using a fairly up-to-date Yoga version (https://github.com/facebook/yoga/tree/c09405d58c624fd26d5e0a7ed98bd10c5cc4c05b from 1.3.2023) while yoga-layout-prebuilt was last updated in 2020 😅

So I assume it's fair that there would be some changes, I'll try to understand what's expected or a bug, or maybe we can fix something within Ink to account for this.

For now, do you think we should revert?

I don't think it's significant enough to revert, it's probably something minor, but would still love to understand what has changed and why 😉 Appreciate your help with this, whenever you have time!