Skip to content

fix: fixes no-typography-in-styles violations [PERA-4132]#382

Merged
fmsouza merged 8 commits intomainfrom
fmsouza/pera-4132-typography
Apr 27, 2026
Merged

fix: fixes no-typography-in-styles violations [PERA-4132]#382
fmsouza merged 8 commits intomainfrom
fmsouza/pera-4132-typography

Conversation

@fmsouza
Copy link
Copy Markdown
Contributor

@fmsouza fmsouza commented Apr 24, 2026

Pull Request Template

Description

  • Fixes all no-typography-in-styles violations in the codebase.

Related Issues

Checklist

  • Have you tested your changes locally?
  • Have you reviewed the code for any potential issues?
  • Have you documented any necessary changes in the project's documentation?
  • Have you added any necessary tests for your changes?
  • Have you updated any relevant dependencies?

Additional Notes

  • Add any additional notes or comments that may be helpful for reviewers.

@fmsouza fmsouza self-assigned this Apr 24, 2026
@fmsouza fmsouza requested review from wjbeau, yasin-ce and yasincaliskan and removed request for yasincaliskan April 24, 2026 14:50

const titleStyle = {
lineHeight: TITLE_LINE_HEIGHT,
flexWrap: 'nowrap' as const,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why as const?

...getTypography(theme, 'body'),
fontWeight: '600' as const,
textTransform: 'none' as const,
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as const?

export const useStyles = makeStyles(theme => {
const activeTitle = {
color: theme.colors.textMain,
fontSize: TITLE_FONT_SIZE,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just use one of the existing styles here?

Comment on lines +17 to +20
const TITLE_FONT_SIZE = 17
const LIST_TITLE_FONT_SIZE = 16
const DATE_VALUE_FONT_SIZE = 16
const DONE_BUTTON_FONT_SIZE = 17
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, do we really need these weird font sizes? I reckon we should just snap to an existing font size for now?

variant='h4'
style={styles.title}
>
Custom Range
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should have these i18n (not necessarily this PR but since you're here...

variant='body'
style={styles.dateLabel}
>
From
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i18n

variant='body'
style={styles.dateLabel}
>
To
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i18n

alignItems: 'center',
gap: theme.spacing.sm,
},
const TAG_FONT_SIZE = 12
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we just use a standard size?

learnMoreText: {
fontSize: 16,
fontWeight: '500',
const LEARN_MORE_FONT_SIZE = 16
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use a standard size here?

marginBottom: theme.spacing.xxs,
},
itemSubtitle: {
const ITEM_SUBTITLE_FONT_SIZE = 12
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

standard size?

fmsouza added 5 commits April 27, 2026 09:06
Replace `as const` annotations with explicit TextStyle typing where
typography props are extracted to dodge no-typography-in-styles. Drop
custom font-size overrides on PWText variants so the variant typography
applies, and remove dead style entries that were never referenced.
@fmsouza fmsouza merged commit 3e0c7a7 into main Apr 27, 2026
8 checks passed
@fmsouza fmsouza deleted the fmsouza/pera-4132-typography branch April 27, 2026 09:20
yasin-ce added a commit that referenced this pull request Apr 27, 2026
Three fixes for runtime/style regressions inherited via the latest
main merge:

- PWText: drop the redundant h1/h2/h3/h4 booleans that were forwarded
  to RNEUI's Text. RNEUI flattens its theme h{n}Style at the END of
  the style chain, which silently overrode any color the caller put
  on `style`. The flags were a no-op for callers using `variant='body'`
  (the default), but PR #382 added `variant='h4'` to PWButton's title
  PWText — making every primary button's title color get stomped to
  textMain. In dark mode buttonPrimaryText (gray[900]) was overridden
  by textMain (gray[100]), producing washed-out text on yellow.
  styles.text already applies the full typography via getTypography,
  so the h{n} flags were both redundant and harmful.

- App.tsx: PR #383 changed the pre-bootstrap loading label from
  <Text> to <PWText>, but PWText's makeStyles needs a ThemeProvider
  in the tree, and the only ThemeProvider lives inside RootComponent
  which only mounts after `bootstrapped && persister`. Crash in
  typography.ts:46 when accessing `theme.colors.textMain`. Wrap the
  pre-bootstrap branch in its own ThemeProvider (using getTheme +
  useIsDarkMode) so PWText has the context it needs while keeping
  the no-primitive-rn-components guardrail satisfied.

- ViewContactScreen styles.ts: replace divider `height: 1` with
  `theme.borders.sm` to satisfy the no-numeric-sizes guardrail
  introduced upstream.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants