[Feature] Update components to use typography tokens or Text component #612

narin · 2024-12-13T02:29:08Z

Description of Change

Updates components to use either typography tokens or the Text component
Typography tokens were used when text required colors that didn't align with tones, text decoration, or special style or props such as flexShrink or allowFontScaling
Note that most components will seem slightly larger due to fractional increase in font size or increased lineHeights
Aliased { Text as RNText } when import from react-native to differentiate between RN component and DS component

Testing Packages

components@0.28.1-alpha.2

Screenshots/Video

Alert Before/after

Button Before/after

Checkbox Before/after

CheckboxGroup Before/after

CheckboxGroup (Tiled) Before/after

Link Before/after

LoadingIndicator Before/after

RadioButton Before/after

SegmentedControl Before/after

Snackbar Before/after

Testing

Tested on iOS
Tested on Android
Tested on Web

PR Checklist

Code reviewer validation:

General
- PR is linked to ticket(s)
- PR has changelog label applied if it's to be included in the changelog
- Acceptance criteria:
  - All satisfied or
  - Documented reason for not being performed or
  - Split to separate ticket and ticket is linked by relevant AC(s)
- Above PR sections adequately filled out
- If any breaking changes, in accordance with the pre-1.0.0 versioning guidelines: a CU ticket has been created for the VA Mobile App detailing necessary adjustments with the package version that will be published by this ticket
Code
- Tests are included if appropriate (or split to separate ticket)
- New functions have proper TSDoc annotations

Publish

If changes warrant a new version per the versioning guidelines and the PR is approved and ready to merge:

Merge main into branch
Merge branch to main
Verify that Check Component Integrations workflow ran successfully
Publish new version after merging

…ext styling - Replaced hardcoded font properties with typography tokens from mobile-tokens. - Simplified textStyle definition and ensured consistent link color handling based on variant. - Removed redundant code related to font settings and link color assignment.

…ext styling - Integrated typography tokens from mobile-tokens to enhance text styling consistency. - Updated textStyle to use typography.vadsFontBodyLarge for better maintainability. - Retained conditional fontFamily assignment for selected state while removing redundant font definitions.

… styling consistency - Replaced hardcoded font properties with typography tokens from mobile-tokens. - Updated text styles for Header, Hint, Error, Label, and Description components to use typography.vadsFontBodyLarge and typography.vadsFontBodySmall. - Renamed Text import to RNText for clarity and consistency across the component. - Removed redundant font definitions to streamline the codebase.

- Removed TODO comments in Button and SegmentedControl components to clean up the codebase. - Updated Link component to set marginBottom to 0

…ext-tokens-or-component

…al cleanup and refactoring: - Simplified getTextStyle function in Button component for cleaner code. - Removed unnecessary Spacer components in CheckboxGroup, RadioButton, and CheckboxRadio components to streamline layout. - Updated LoadingIndicator to remove unused imports and improve clarity. - Refactored FormText component to utilize typography tokens for consistent text styling across Header, Hint, Error, Label, and Description components. - Replaced hardcoded font properties with typography tokens and integrated the Text component for improved maintainability.

…or-component' of github.com:department-of-veterans-affairs/va-mobile-library into feature/542-narin-update-components-to-use-text-tokens-or-component

TimRoe · 2024-12-16T16:41:40Z

Should this also be linked to another ticket? Just linked to one for adding colors, not updating components to use Text.

TimRoe · 2024-12-16T17:08:22Z

packages/components/src/components/Alert/Alert.tsx

@@ -143,8 +128,9 @@ export const Alert: FC<AlertProps> = ({
  const iconViewStyle: ViewStyle = {
    flexDirection: 'row',
    // Below keeps icon aligned with first row of text, centered, and scalable
+    // If Text variant for header changes, token referenced in minHeight must change accordingly


Not sure if there's any good way to rework, but seems unlikely this comment would be noticed in that eventuality.

TimRoe · 2024-12-16T17:14:13Z

packages/components/src/components/Button/Button.tsx

+  const getTextStyle = (pressed: boolean): TextStyle => ({
+    ...typography.vadsFontBodyLarge,
+    fontFamily: family.vadsFontFamilySansSerifBold,
+    marginBottom: 0,


Should maybe use the none spacing token.

TimRoe · 2024-12-16T17:16:22Z

packages/components/src/components/SegmentedControl/SegmentedControl.tsx

-      ...font,
+      ...typography.vadsFontBodyLarge,
+      fontFamily: isSelected ? 'SourceSansPro-Bold' : 'SourceSansPro-Regular',
+      marginBottom: 0,


Similar token suggestion.

packages/components/src/components/Text/Text.stories.tsx

TimRoe · 2024-12-16T17:26:12Z

packages/components/src/components/SegmentedControl/SegmentedControl.tsx

-      ...font,
+      ...typography.vadsFontBodyLarge,
+      fontFamily: isSelected ? 'SourceSansPro-Bold' : 'SourceSansPro-Regular',
+      marginBottom: 0,
      color: theme.vadsColorForegroundDefault,


Did none of the tones work here? Seems like with the Text updates to include bold and center that maybe this could still use the Text component?

Couldn't use Text component here due to the allowFontScaling prop used on line 121, unless we add that prop.

packages/components/src/components/Text/Text.tsx

packages/components/src/components/shared/FormText.tsx

TimRoe · 2024-12-16T18:03:32Z

packages/components/src/components/shared/FormText.tsx

+    ...typography.vadsFontBodyLarge,
+    marginBottom: 0,
    color: theme.vadsColorForegroundDefault,


Can't this be handled by our Text component with default tone and none bottomSpacing?

Oh, or can it not because the role of heading while being body text?

Yup. This was done due to the role

packages/components/src/components/shared/FormText.tsx

TimRoe

Approving since the whole seems alright, but some comments that likely should be addressed prior to merging.

…r Label and required text

…ext-tokens-or-component

TimRoe · 2024-12-16T19:46:26Z

packages/components/src/components/Text/Text.tsx

@@ -79,6 +79,9 @@ export const Text: FC<TextProps> = ({
  variant = 'body',
 }) => {
  const theme = useTheme()
+
+  if (!children) return null


Was this needed for TS to not get upset about the bold check or just a random piece of "defensive" code?

TimRoe · 2024-12-16T19:48:40Z

packages/components/src/components/shared/FormText.tsx

 import { useTranslation } from 'react-i18next'
 import React, { FC } from 'react'

+import { SpacerSize } from '../Spacer/Spacer'


Calling attention to this since I assume it's going to cause linting failures in main.

narin added 11 commits December 11, 2024 13:31

Update button to use typography token

eee3217

Update Alert to use typography tokens

60d3ac9

Rename Text import to RNText

e90b806

Update LoadingIndicator to use Text component

2ce35bb

Update Snackbar to use typography tokens

dd65d97

Add bold boolean for body variant

2e615d0

Enhance component styling and parameters

5402a2d

- Removed TODO comments in Button and SegmentedControl components to clean up the codebase. - Updated Link component to set marginBottom to 0

Reset default viewport

419a80b

narin changed the title ~~Feature/542 narin update components to use text tokens or component~~ [Feature] Update components to use typography tokens or Text component Dec 13, 2024

va-mobile-automation-robot and others added 7 commits December 13, 2024 05:58

Version bump: components-v0.28.1-alpha.2

3b74dde

Merge branch 'main' into feature/542-narin-update-components-to-use-t…

3c2c278

…ext-tokens-or-component

Merge branch 'feature/542-narin-update-components-to-use-text-tokens-…

56d4301

…or-component' of github.com:department-of-veterans-affairs/va-mobile-library into feature/542-narin-update-components-to-use-text-tokens-or-component

Replace RNText in Alert with Text component

3ebba55

Remove unused vars

ed670bd

Rename Text import to RNText

9fdfbc2

narin marked this pull request as ready for review December 13, 2024 20:48

narin requested a review from a team as a code owner December 13, 2024 20:48

narin added 4 commits December 13, 2024 12:56

Revert FormText spacers

bd41f7c

Fix whitespace

270ab94

Add comment for center prop

0b1e806

Add comment about icon alignment

55f8107

TimRoe added the changelog label Dec 16, 2024

TimRoe reviewed Dec 16, 2024

View reviewed changes

packages/components/src/components/Text/Text.stories.tsx Show resolved Hide resolved

TimRoe reviewed Dec 16, 2024

View reviewed changes

packages/components/src/components/Text/Text.tsx Outdated Show resolved Hide resolved

TimRoe reviewed Dec 16, 2024

View reviewed changes

packages/components/src/components/shared/FormText.tsx Outdated Show resolved Hide resolved

TimRoe reviewed Dec 16, 2024

View reviewed changes

packages/components/src/components/shared/FormText.tsx Outdated Show resolved Hide resolved

narin added 2 commits December 16, 2024 10:32

Update Text story argTypes comment to be more generic

88a50b1

Simplify bold conditional

6f5c016

TimRoe approved these changes Dec 16, 2024

View reviewed changes

narin and others added 3 commits December 16, 2024 10:33

Remove fonts section comment

8a37721

Use spacing token instead of marginBottom: 0. Use common textProps fo…

46322b7

…r Label and required text

Merge branch 'main' into feature/542-narin-update-components-to-use-t…

7df1aad

…ext-tokens-or-component

narin merged commit 5a50058 into main Dec 16, 2024
4 of 5 checks passed

narin deleted the feature/542-narin-update-components-to-use-text-tokens-or-component branch December 16, 2024 19:34

TimRoe reviewed Dec 16, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature] Update components to use typography tokens or Text component #612

[Feature] Update components to use typography tokens or Text component #612

narin commented Dec 13, 2024 •

edited by TimRoe

Loading

TimRoe commented Dec 16, 2024

TimRoe Dec 16, 2024

TimRoe Dec 16, 2024

TimRoe Dec 16, 2024

TimRoe Dec 16, 2024

narin Dec 16, 2024

TimRoe Dec 16, 2024

TimRoe Dec 16, 2024

narin Dec 16, 2024

TimRoe left a comment

TimRoe Dec 16, 2024

TimRoe Dec 16, 2024

[Feature] Update components to use typography tokens or Text component #612

[Feature] Update components to use typography tokens or Text component #612

Conversation

narin commented Dec 13, 2024 • edited by TimRoe Loading

Description of Change

Testing Packages

Screenshots/Video

Testing

PR Checklist

Publish

TimRoe commented Dec 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TimRoe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

narin commented Dec 13, 2024 •

edited by TimRoe

Loading