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

Support for alignItems: baseline for <Text> elements in Fabric #31639

Closed
wants to merge 2 commits into from

Conversation

shergin
Copy link
Contributor

@shergin shergin commented Jun 1, 2021

Summary

This implements alignItems: baseline in Fabric for both platforms.

This fixes #20666 and #21918 and others, see also #31575 and 51b3529.

Changelog

[Core][Fixed] - Support for alignItems: baseline for elements in Fabric

Test Plan

Screen Shot 2021-05-31 at 11 18 55 PM

Screen Shot 2021-05-23 at 9 55 20 PM

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 1, 2021
@yungsters yungsters requested review from mdvacca and removed request for mdvacca June 1, 2021 04:35
@facebook-github-bot
Copy link
Contributor

@yungsters has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@yungsters has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@pull-bot
Copy link

pull-bot commented Jun 1, 2021

Messages
📖

📋 Verify Changelog Format - A changelog entry has the following format: [CATEGORY] [TYPE] - Message.

CATEGORY may be:
  • General
  • iOS
  • Android
  • JavaScript
  • Internal (for changes that do not need to be called out in the release notes)

TYPE may be:

  • Added, for new features.
  • Changed, for changes in existing functionality.
  • Deprecated, for soon-to-be removed features.
  • Removed, for now removed features.
  • Fixed, for any bug fixes.
  • Security, in case of vulnerabilities.

MESSAGE may answer "what and why" on a feature level. Use this to briefly tell React Native users about notable changes.

Generated by 🚫 dangerJS against edbece8

@analysis-bot
Copy link

analysis-bot commented Jun 1, 2021

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 4090195

@yungsters yungsters self-assigned this Jul 28, 2021
@yungsters
Copy link
Contributor

I am curious what tradeoffs you considered when introducing a bag of values named extras. Why not make this baseline instead? If we want to generalize a pattern here, is there any way in which we could generalize the baseline and attachmentsPositions, maybe?

@shergin
Copy link
Contributor Author

shergin commented Sep 3, 2021

Just wanted to make it extensible (considering how many hoops we need to jump to pass values across multiple indirection layers). It's tricky to maintain a balance between extensibility, performance, simplicity, and clean looking code here. My assumption was that we will probably need to add more numeric text metrics in the future, so I called it extras to make it simpler in the future.

Of course we can rename that to baseline.

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,708,090 +91
android hermes armeabi-v7a 7,238,800 +655
android hermes x86 8,128,752 +358
android hermes x86_64 8,093,626 +125
android jsc arm64-v8a 9,627,528 +10
android jsc armeabi-v7a 8,545,762 +566
android jsc x86 9,642,462 +255
android jsc x86_64 10,251,017 +21

Base commit: 4090195

@yungsters
Copy link
Contributor

To clarify next steps here, I think we should rename to baseline for now to avoid over-engineering and so that future use cases can be more thoughtfully generalized.

@github-actions
Copy link

This PR is waiting for author's feedback since 24 days. Please provide the requested feedback or this will be closed in 7 days

@github-actions github-actions bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Feb 11, 2023
@github-actions
Copy link

This PR was closed because the author hasn't provided the requested feedback after 7 days.

@github-actions github-actions bot closed this Feb 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Needs: Author Feedback Stale There has been a lack of activity on this issue and it may be closed soon.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

flex alignItems "baseline" not aligning consistently like on web
7 participants