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

Update public API for intrinsic sizing setters #46939

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

joevilches
Copy link
Contributor

Summary:
X-link: facebook/yoga#1722

tsia! opted for one function for each keyword just like auto. This is kinda annoying and not the most sustainable, so maybe it makes more sense to make a new enum here and just add one function

Differential Revision: D64002837

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner labels Oct 10, 2024
@facebook-github-bot
Copy link
Contributor

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

joevilches added a commit to joevilches/yoga that referenced this pull request Oct 10, 2024
Summary:
X-link: facebook/react-native#46939


tsia! opted for one function for each keyword just like auto. This is kinda annoying and not the most sustainable, so maybe it makes more sense to make a new enum here and just add one function

Differential Revision: D64002837
joevilches added a commit to joevilches/react-native that referenced this pull request Oct 10, 2024
Summary:

X-link: facebook/yoga#1722

tsia! opted for one function for each keyword just like auto. This is kinda annoying and not the most sustainable, so maybe it makes more sense to make a new enum here and just add one function

Differential Revision: D64002837
@facebook-github-bot
Copy link
Contributor

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

joevilches added a commit to joevilches/yoga that referenced this pull request Oct 29, 2024
Summary:
X-link: facebook/react-native#46939


tsia! opted for one function for each keyword just like auto. This is kinda annoying and not the most sustainable, so maybe it makes more sense to make a new enum here and just add one function

Reviewed By: NickGerleman

Differential Revision: D64002837
joevilches added a commit to joevilches/react-native that referenced this pull request Oct 29, 2024
Summary:

X-link: facebook/yoga#1722

tsia! opted for one function for each keyword just like auto. This is kinda annoying and not the most sustainable, so maybe it makes more sense to make a new enum here and just add one function

Reviewed By: NickGerleman

Differential Revision: D64002837
@facebook-github-bot
Copy link
Contributor

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

joevilches added a commit to joevilches/yoga that referenced this pull request Oct 30, 2024
Summary:
X-link: facebook/react-native#46939


tsia! opted for one function for each keyword just like auto. This is kinda annoying and not the most sustainable, so maybe it makes more sense to make a new enum here and just add one function

Reviewed By: NickGerleman

Differential Revision: D64002837
joevilches added a commit to joevilches/react-native that referenced this pull request Oct 30, 2024
Summary:

X-link: facebook/yoga#1722

tsia! opted for one function for each keyword just like auto. This is kinda annoying and not the most sustainable, so maybe it makes more sense to make a new enum here and just add one function

Reviewed By: NickGerleman

Differential Revision: D64002837
@facebook-github-bot
Copy link
Contributor

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

joevilches added a commit to joevilches/yoga that referenced this pull request Oct 31, 2024
Summary:
X-link: facebook/react-native#46939


tsia! opted for one function for each keyword just like auto. This is kinda annoying and not the most sustainable, so maybe it makes more sense to make a new enum here and just add one function

Reviewed By: NickGerleman

Differential Revision: D64002837
joevilches added a commit to joevilches/react-native that referenced this pull request Oct 31, 2024
Summary:

X-link: facebook/yoga#1722

tsia! opted for one function for each keyword just like auto. This is kinda annoying and not the most sustainable, so maybe it makes more sense to make a new enum here and just add one function

Reviewed By: NickGerleman

Differential Revision: D64002837
@facebook-github-bot
Copy link
Contributor

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

joevilches added a commit to joevilches/yoga that referenced this pull request Oct 31, 2024
Summary:
X-link: facebook/react-native#46939


tsia! opted for one function for each keyword just like auto. This is kinda annoying and not the most sustainable, so maybe it makes more sense to make a new enum here and just add one function

Reviewed By: NickGerleman

Differential Revision: D64002837
joevilches added a commit to joevilches/react-native that referenced this pull request Oct 31, 2024
Summary:

X-link: facebook/yoga#1722

tsia! opted for one function for each keyword just like auto. This is kinda annoying and not the most sustainable, so maybe it makes more sense to make a new enum here and just add one function

Reviewed By: NickGerleman

Differential Revision: D64002837
@facebook-github-bot
Copy link
Contributor

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

… keywords (facebook#46938)

Summary:
X-link: facebook/yoga#1721


The private internals of how we store styles needed to change a bit to support 3 new keyword values. Right now the only other keyword that can be stored is `auto`. As a result there isn't much fancy logic to support storing this and its just stored as a specific type inside of `StyleValueHandle`. There are only 3 bits for types (8 values), so it is not sustainable to just stuff every keyword in there. So the change writes the keyword as a value with a new `keyword` `Type`. 

I chose not to put `auto` in there even though it is a keyword since it is a hot path, I did not want to regress perf when I did not need to.

I also make a new `StyleSizeValue` class to store size values - so values for `width`, `height`, etc. This way these new keywords are kept specific to sizes and we will not be able to create, for example, a margin: `max-content`.

Reviewed By: NickGerleman

Differential Revision: D63927512
Summary:

X-link: facebook/yoga#1722

tsia! opted for one function for each keyword just like auto. This is kinda annoying and not the most sustainable, so maybe it makes more sense to make a new enum here and just add one function

Reviewed By: NickGerleman

Differential Revision: D64002837
joevilches added a commit to joevilches/yoga that referenced this pull request Nov 1, 2024
Summary:
X-link: facebook/react-native#46939


tsia! opted for one function for each keyword just like auto. This is kinda annoying and not the most sustainable, so maybe it makes more sense to make a new enum here and just add one function

Reviewed By: NickGerleman

Differential Revision: D64002837
@facebook-github-bot
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported p: Facebook Partner: Facebook Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants