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 #1722

Closed
wants to merge 2 commits into from

Conversation

joevilches
Copy link
Contributor

Summary: 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

Copy link

vercel bot commented Oct 10, 2024

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 Nov 4, 2024 7:36pm

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. 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
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

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
joevilches added a commit to joevilches/react-native that referenced this pull request Nov 1, 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/react-native that referenced this pull request Nov 4, 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

Changelog: [Internal]

Reviewed By: NickGerleman

Differential Revision: D64002837
joevilches added a commit to joevilches/yoga that referenced this pull request Nov 4, 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

Changelog: [Internal]

Reviewed By: NickGerleman

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

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

…acebook#1721)

Summary:

X-link: facebook/react-native#46938

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`.

Changelog: [Internal]

Reviewed By: NickGerleman

Differential Revision: D63927512
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

Changelog: [Internal]

Reviewed By: NickGerleman

Differential Revision: D64002837
joevilches added a commit to joevilches/react-native that referenced this pull request Nov 4, 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

Changelog: [Internal]

Reviewed By: NickGerleman

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

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

facebook-github-bot pushed a commit to facebook/litho that referenced this pull request Nov 5, 2024
Summary:
X-link: facebook/react-native#46939

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

Changelog: [Internal]

Reviewed By: NickGerleman

Differential Revision: D64002837

fbshipit-source-id: f15fae9fc0103175e1d85850fc9aa68579989fd3
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 488288e.

facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Nov 5, 2024
Summary:
Pull Request resolved: #46939

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

Changelog: [Internal]

Reviewed By: NickGerleman

Differential Revision: D64002837

fbshipit-source-id: f15fae9fc0103175e1d85850fc9aa68579989fd3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants