DaddyFrosty/yoga
1
0
Fork 0
Code Issues 96 Pull Requests 31 Actions Packages Projects Releases 33 Wiki Activity

Fix test util to measure text properly based on flex direction #1768

Closed
joevilches wants to merge 1 commits from export-D67106199 into main
pull from: export-D67106199
merge into: DaddyFrosty:main
Conversation 7 Commits 1 Files Changed 11 +216 -37
joevilches commented 2024-12-11 12:45:51 -08:00 (Migrated from github.com)
Copy Link

Summary: Depending on the flex direction text will either be capped to the measured size or to the longest word, so I added that functionality

Differential Revision: D67106199

Summary: Depending on the flex direction text will either be capped to the measured size or to the longest word, so I added that functionality Differential Revision: D67106199
vercel[bot] commented 2024-12-11 12:45:55 -08:00 (Migrated from github.com)
Copy Link

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
yoga-website ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 11, 2024 9:40pm
[vc]: #DpfoOjSjyaf8cggo/eYMs1T+mG2jyBlW+0ETxvAdSr8=:eyJpc01vbm9yZXBvIjp0cnVlLCJ0eXBlIjoiZ2l0aHViIiwicHJvamVjdHMiOlt7Im5hbWUiOiJ5b2dhLXdlYnNpdGUiLCJyb290RGlyZWN0b3J5Ijoid2Vic2l0ZSIsImluc3BlY3RvclVybCI6Imh0dHBzOi8vdmVyY2VsLmNvbS9mYm9wZW5zb3VyY2UveW9nYS13ZWJzaXRlL0dZUHBkdVVSQldtMWlveXc5dmJlN05FWnliY0giLCJwcmV2aWV3VXJsIjoieW9nYS13ZWJzaXRlLWdpdC1mb3JrLWpvZXZpbGNoZXMtZXhwb3J0LWQ2NzEwNjE5OS1mYm9wZW5zb3VyY2UudmVyY2VsLmFwcCIsIm5leHRDb21taXRTdGF0dXMiOiJERVBMT1lFRCIsImxpdmVGZWVkYmFjayI6eyJyZXNvbHZlZCI6MCwidW5yZXNvbHZlZCI6MCwidG90YWwiOjAsImxpbmsiOiJ5b2dhLXdlYnNpdGUtZ2l0LWZvcmstam9ldmlsY2hlcy1leHBvcnQtZDY3MTA2MTk5LWZib3BlbnNvdXJjZS52ZXJjZWwuYXBwIn19XX0= **The latest updates on your projects**. Learn more about [Vercel for Git ↗︎](https://vercel.link/github-learn-more) | Name | Status | Preview | Comments | Updated (UTC) | | :--- | :----- | :------ | :------- | :------ | | **yoga-website** | ✅ Ready ([Inspect](https://vercel.com/fbopensource/yoga-website/GYPpduURBWm1ioyw9vbe7NEZybcH)) | [Visit Preview](https://yoga-website-git-fork-joevilches-export-d67106199-fbopensource.vercel.app) | 💬 [**Add feedback**](https://vercel.live/open-feedback/yoga-website-git-fork-joevilches-export-d67106199-fbopensource.vercel.app?via=pr-comment-feedback-link) | Dec 11, 2024 9:40pm |
facebook-github-bot commented 2024-12-11 14:04:51 -08:00 (Migrated from github.com)
Copy Link

This pull request was exported from Phabricator. Differential Revision: D67106199

This pull request was **exported** from Phabricator. Differential Revision: [D67106199](https://www.internalfb.com/diff/D67106199)
facebook-github-bot commented 2024-12-11 14:05:05 -08:00 (Migrated from github.com)
Copy Link

This pull request was exported from Phabricator. Differential Revision: D67106199

This pull request was **exported** from Phabricator. Differential Revision: [D67106199](https://www.internalfb.com/diff/D67106199)
facebook-github-bot commented 2024-12-12 11:15:10 -08:00 (Migrated from github.com)
Copy Link

This pull request has been merged in facebook/yoga@909e4bea6e.

This pull request has been merged in facebook/yoga@909e4bea6e6764e4028d5e47b2f67486e6d26c7e.
nicoburns commented 2024-12-14 19:28:26 -08:00 (Migrated from github.com)
Copy Link

@joevilches I don't think this is right. I believe the measure function should still return the size floored by the longest word size, but the parent container should override that size due to the max-width style (which probably is different for flexbox row vs. column due to Flexbox having a weird quirk of not applying min/max sizes to children when determining the flex base size in the main axis). As a general principle, the min- or max- content size of box is never affected by the parent's display mode (although the resultant size of the box may be).

@joevilches I don't think this is right. I believe the measure function should still return the size floored by the longest word size, but the parent container should override that size due to the max-width style (which probably is different for flexbox row vs. column due to Flexbox having a weird quirk of not applying min/max sizes to children when determining the flex base size in the main axis). As a general principle, the min- or max- content size of box is never affected by the parent's display mode (although the resultant size of the box may be).
joevilches commented 2024-12-16 10:17:59 -08:00 (Migrated from github.com)
Copy Link

@nicoburns that makes sense thanks for commenting. I am curious how Yoga would yield the same results as chrome in this case, since it did seem like at some point we would need to measure with some smaller width than the longest word to get the correct height. I can take a look again, maybe the issue is something else.

@nicoburns that makes sense thanks for commenting. I am curious how Yoga would yield the same results as chrome in this case, since it did seem like at some point we would need to measure with some smaller width than the longest word to get the correct height. I can take a look again, maybe the issue is something else.
nicoburns commented 2024-12-23 17:21:29 -08:00 (Migrated from github.com)
Copy Link

@joevilches I think I previously misunderstood this code (not helped by the fact I was trying to read it on my phone!). The width you are using does seem broadly correct to me with default styles. Although the way we deal with this in Taffy is by using (what Yoga calls) an "Exact" sizing constraint in the column case (depending on align-items)

I think the reason this is (/seems to be) different for columns vs. rows is because align-items defaults to stretch. Which will mean that with default styles the min-content width will overridden for a column. I believe this wouldn't apply with align-items overridden to start or similar.

The following HTML:

<div style="display: flex;width: 50px;flex-direction: column;">
        <div>
            orheg irehg ireh girehg iorhioh iroegh 
            jhnreuighuirebghuierhguierbuigbreuibuebgiuerbgiuberuigberuibguirebg
            righierhg ihe irogh eriohg iorehg ioerhg iorehg 
        </div>
</div>

Results in:

Screenshot 2024-12-24 at 14 19 23

But if you add an align-items: start style to the outer div then you get:

Screenshot 2024-12-24 at 14 19 13
@joevilches I think I previously misunderstood this code (not helped by the fact I was trying to read it on my phone!). The width you are using does seem broadly correct to me with default styles. Although the way we deal with this in Taffy is by using (what Yoga calls) an "Exact" sizing constraint in the column case (depending on `align-items`) I think the reason this is (/seems to be) different for columns vs. rows is because `align-items` defaults to `stretch`. Which will mean that with default styles the min-content width will overridden for a column. I believe this wouldn't apply with `align-items` overridden to `start` or similar. The following HTML: ```html <div style="display: flex;width: 50px;flex-direction: column;"> <div> orheg irehg ireh girehg iorhioh iroegh jhnreuighuirebghuierhguierbuigbreuibuebgiuerbgiuberuigberuibguirebg righierhg ihe irogh eriohg iorehg ioerhg iorehg </div> </div> ``` Results in: <img width="486" alt="Screenshot 2024-12-24 at 14 19 23" src="https://github.com/user-attachments/assets/f3dc1d3b-0de8-4e92-a918-9a49a8bbf324" /> But if you add an `align-items: start` style to the outer div then you get: <img width="486" alt="Screenshot 2024-12-24 at 14 19 13" src="https://github.com/user-attachments/assets/b3ad91e8-1dcc-429e-ac36-bd232e5e940b" />

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
Algorithm
Build and Infrastructure
C#
C++ Enhancement
CLA Signed
Discussion
Documentation
Feature Request
Java
JavaScript
Merged
Question
Reverted
Shared with Meta
Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
YogaKit
YogaLayout ViewGroup
bug
dependencies
Pull requests that update a dependency file
duplicate
enhancement
fb-exported
fh:direct-merge-enabled
invalid
ruby
Pull requests that update Ruby code
wontfix
No Label
CLA Signed
Merged
fb-exported
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
DaddyFrosty
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: DaddyFrosty/yoga#1768
No description provided.
Delete Branch "export-D67106199"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?