Skip to content

1/n Nested Text textAlignVertical (Android) - Adding CustomStyleSpan API to align nested text vertically - #35949

Closed
fabOnReact wants to merge 107 commits into
react:mainfrom
fabOnReact:span-api-junit-tests
Closed

1/n Nested Text textAlignVertical (Android) - Adding CustomStyleSpan API to align nested text vertically#35949
fabOnReact wants to merge 107 commits into
react:mainfrom
fabOnReact:span-api-junit-tests

Conversation

@fabOnReact

@fabOnReactfabOnReact commented Jan 24, 2023

Copy link
Copy Markdown
Contributor

Summary

1/n of a series of pull requests (Nested Text textAlignVertical).

The CSS property vertically aligns the text relative to the lineHeight (comment).
https://developer.mozilla.org/en-US/docs/Web/CSS/vertical-align

The vertical-align CSS property sets the vertical alignment of an inline, inline-block, or table-cell box.

This PR adds the option textAlignVertical in the CustomStyleSpan constructor to change the text baseline (vertical alignment). The updateDrawState method is responsible for aligning the span (text) to the top or bottom.

  • If the text has multiple spans with different line heights, it applies the highest line height (the default behavior without inline images).
  • The text is aligned based on the font metrics top, bottom, ascent, and descent and the line height
  • Text FontMetrics are updated when fontSize changes and the text remains aligned to the top/bottom or center
  • Multiple spans with different font sizes correctly align to the top/bottom

Update 2nd of February

  • Moved the vertical text-align logic from ReactAbsoluteSizeSpan to CustomStyleSpan.
  • Investigate the cpp implementation of Paint.cpp
  • Use TextPaint API to calculate FontMetrics with different font-sizes
  • Mock the TextPaint constructor with PowerMock to fix JUnit tests.

Next PR: 2/n Nested Text textAlignVertical (Android) - adding textAlignVertical prop to NestedText #35704

Related:

fixes#30375#30375 (comment)

Changelog

[ANDROID] [ADDED] - Adding CustomStyleSpan API to align nested text vertically

Test Plan

Video tests of the functionality: #35949 (comment) and #35949 (comment)
Previous Video tests of the functionality in PR #35704
#35949 (comment)
#35704 (comment)
#35704 (comment)
#35704 (comment)

Previous Tests

#35704 (comment)
#35704 (comment)#35704 (comment)

@facebook-github-botfacebook-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 Jan 24, 2023
@fabOnReact

fabOnReact commented Jan 24, 2023

Copy link
Copy Markdown
ContributorAuthor

94c5da5

Ptypetask
1featureRun CustomLineHeight test on circleci
1.1taskRun test on circleci for CustomLineHeight without failure
1.2taskTrigger CustomLineHeight failure in CircleCi (log)

P type task
1 feature Run CustomLineHeight test on circleci
1.1 task Run test on circleci for CustomLineHeight without failure
1.2 task Trigger CustomLineHeight failure in CircleCi (log)

@analysis-bot

analysis-bot commented Jan 24, 2023

Copy link
Copy Markdown
PlatformEngineArchSize (bytes)Diff
androidhermesarm64-v8a8,453,797-4,837
androidhermesarmeabi-v7a7,777,311-4,584
androidhermesx868,930,030-4,505
androidhermesx86_648,787,255-4,369
androidjscarm64-v8a6,668,475-2,424,297
androidjscarmeabi-v7a7,462,453-828,416
androidjscx869,192,588+48,998
androidjscx86_646,893,602-2,508,853

Base commit: 26b2bb5
Branch: main

fabOnReact added a commit to fabOnReact/react-native that referenced this pull request Feb 9, 2023
@fabOnReact

Copy link
Copy Markdown
ContributorAuthor
Ptypetask
1reviewTest functionality with different types of font styles
2bugText does not align correctly to the top when moving logic to CustomStyleSpan
2.1taskDebug the value of fontSize in CustomStyleSpan with fontSize 32. Record video (video).
2.2taskDebug the value of getSize in ReactAbsSizeSpan with fontSize 32. Record video (video).

P type task
1 review Test functionality with different types of font styles
2 bug Text does not align correctly to the top when moving logic to CustomStyleSpan
2.1 task Debug the value of fontSize in CustomStyleSpan with fontSize 32. Record video (video).
2.2 task Debug the value of getSize in ReactAbsSizeSpan with fontSize 32. Record video (video).

@fabOnReact

fabOnReact commented Feb 9, 2023

Copy link
Copy Markdown
ContributorAuthor

fabOnReact@112eeeefabOnReact@d8556a7

Ptypetask
3bugThe parent Text does not align correctly with CustomStyleSpan with Gravity.TOP (correct-behaviour, not-correct-behaviour, commit)
3.1taskChange parent text alignment (Gravity.TOP to BOTTOM)
4bugThe logic for the highestFontSize correctly aligns the text. The issue is the remaining logic from CustomStyleSpan applied to other Spans.
4.1taskComment the remaining logic and understand the root cause of the problem.

P type task
3 bug The parent Text does not align correctly with CustomStyleSpan with Gravity.TOP (correct-behaviour, not-correct-behaviour, commit)
3.1 task Change parent text alignment (Gravity.TOP to BOTTOM)
4 bug The logic for the highestFontSize correctly aligns the text. The issue is the remaining logic from CustomStyleSpan applied to other Spans.
4.1 task Comment the remaining logic and understand the root cause of the problem.

fabOnReact added a commit to fabOnReact/react-native that referenced this pull request Feb 9, 2023
…avity.TOP (correct-behaviour, not-correct-behaviour, commit)
react#35949 (comment)
@fabOnReact

fabOnReact commented Feb 10, 2023

Copy link
Copy Markdown
ContributorAuthor

@fabOnReact

fabOnReact commented Feb 10, 2023

Copy link
Copy Markdown
ContributorAuthor
Correctly aligning top/bottom with a different font family/style

https://www.icloud.com/iclouddrive/08ekjoNTUTIhPIY3nCF1C83mQ#using_different_font_style

@fabOnReactfabOnReact changed the title 1/n Nested Text textAlignVertical (Android) - Adding ReactAbsoluteSizeSpan API to align nested text vertically1/n Nested Text textAlignVertical (Android) - Adding CustomStyleSpan API to align nested text verticallyFeb 10, 2023
@fabOnReact
fabOnReact marked this pull request as ready for review February 10, 2023 11:40
fabOnReact added a commit to fabOnReact/react-native that referenced this pull request Feb 20, 2023
@github-actions

Copy link
Copy Markdown

This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actionsgithub-actionsBot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Jan 17, 2024
@github-actions

Copy link
Copy Markdown

This PR was closed because it has been stalled for 7 days with no activity.

@fabOnReact

Copy link
Copy Markdown
ContributorAuthor

Should not have been closed.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.Shared with MetaApplied via automation to indicate that an Issue or Pull Request has been shared with the team.StaleThere 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.

[Android] textAlignVertical doesn't work on nested text

4 participants

@fabOnReact@analysis-bot@NickGerleman@facebook-github-bot