Skip to content

gpui: Bump taffy to 0.4.3 again#11655

Merged
osiewicz merged 4 commits intomainfrom
bump-taffy-version
May 10, 2024
Merged

gpui: Bump taffy to 0.4.3 again#11655
osiewicz merged 4 commits intomainfrom
bump-taffy-version

Conversation

@osiewicz
Copy link
Copy Markdown
Member

@osiewicz osiewicz commented May 10, 2024

We reverted bump to taffy 0.4.3 following an issue spotted by @maxdeviant where chat text input was not being rendered correctly:
image
This was an issue with the previous attempt to upgrade to taffy 0.4.0 as well. We bail early in compute_auto_height_layout due to a missing width:

let width = known_dimensions.width?;

The same issue is visible in story for auto-height editor (or rather, the breakage is visible - the editor simply does not render at all there).

I tracked down the breakage to DioxusLabs/taffy#573 ; it looks like it specifically affects editors with auto-height. In taffy <0.4 which we were using previously, we'd eventually get a proper width for auto-height EditorElement after having initially computed the size. With taffy 0.4 however (and specifically that PR mentioned earlier), we're getting Size::NONE in layout phase 1.
I've noticed though that even with taffy <0.3, the known_dimensions.width was always equal to available_space.width in layout phase. Hence, I went with falling back to available_space.width when it is a definite value and we don't have a known_dimensions.width.
Done this way, both chat input and auto-height story render correctly.
/cc @as-cii
Related:
#11606
#11622
#7868
#7896

Release Notes:

  • N/A

Footnotes

  1. This could possibly be related to change in what gets passed in https://github.com/DioxusLabs/taffy/pull/573/files#diff-60c916e9b0c507925f032cecdde6ae163e41b84b8e4bc0a6c04f7d846b0aad9eR133 , though I'm not sure if editor is a leaf node in this case

osiewicz added 3 commits May 9, 2024 12:30
Taffy 0.4 has been released 2 months ago. We've been using an older commit from their 0.4 development branch since November.
@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label May 10, 2024
@as-cii
Copy link
Copy Markdown
Member

as-cii commented May 10, 2024

Looks reasonable! 👍

@osiewicz osiewicz merged commit df00854 into main May 10, 2024
@osiewicz osiewicz deleted the bump-taffy-version branch May 10, 2024 13:05
osiewicz added a commit to RemcoSmitsDev/zed that referenced this pull request May 18, 2024
We reverted bump to taffy 0.4.3 following an issue spotted by
@maxdeviant where chat text input was not being rendered correctly:

![image](https://github.com/zed-industries/zed/assets/24362066/9d7e6444-47b1-4ac2-808f-bf10404377c0)
This was an issue with the previous attempt to upgrade to taffy 0.4.0 as
well. We bail early in `compute_auto_height_layout` due to a missing
width:
https://github.com/zed-industries/zed/blob/df190ea84621837c44fa50c62837bdbea04b9e22/crates/editor/src/element.rs#L5266
The same issue is visible in story for auto-height editor (or rather,
the breakage is visible - the editor simply does not render at all
there).

I tracked down the breakage to
DioxusLabs/taffy#573 ; it looks like it
specifically affects editors with auto-height. In taffy <0.4 which we
were using previously, we'd eventually get a proper width for
auto-height EditorElement after having initially computed the size. With
taffy 0.4 however (and specifically that PR mentioned earlier), we're
getting `Size::NONE` in layout phase [^1].
I've noticed though that even with taffy <0.3, the
`known_dimensions.width` was always equal to `available_space.width` in
layout phase. Hence, I went with falling back to `available_space.width`
when it is a definite value and we don't have a
`known_dimensions.width`.
Done this way, both chat input and auto-height story render correctly.
/cc @as-cii 
Related:
zed-industries#11606
zed-industries#11622
zed-industries#7868
zed-industries#7896

[^1]: This could possibly be related to change in what gets passed in
https://github.com/DioxusLabs/taffy/pull/573/files#diff-60c916e9b0c507925f032cecdde6ae163e41b84b8e4bc0a6c04f7d846b0aad9eR133
, though I'm not sure if editor is a leaf node in this case

Release Notes:

- N/A
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed The user has signed the Contributor License Agreement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants