Skip to content

feat(typography): support dynamic type - #28045

Merged
brandyscarney merged 3 commits into
FW-4146from
FW-4928
Aug 24, 2023
Merged

feat(typography): support dynamic type#28045
brandyscarney merged 3 commits into
FW-4146from
FW-4928

Conversation

@brandyscarney

@brandyscarney brandyscarney commented Aug 22, 2023

Copy link
Copy Markdown
Member

Issue number: N/A


What is the current behavior?

The styles for native headings do not support dynamic type.

What is the new behavior?

  • Adds support for dynamic type by converting font-size to use rem units
  • Adds test for scaling the font in core/src/css/test/a11y
    • I thought this made more sense than putting it in a component folder like text since core/src/css is where the styles exist, but maybe we don't need the additional a11y folder since we could just keep all related tests in one file
  • Updates the sass version to ^1.33.0

Does this introduce a breaking change?

  • Yes
  • No

@bolt-new-by-stackblitz

Copy link
Copy Markdown

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@github-actions github-actions Bot added the package: core @ionic/core package label Aug 22, 2023
Comment thread core/package.json
"prettier": "^2.6.1",
"rollup": "^2.26.4",
"sass": "^1.26.10",
"sass": "^1.33.0",

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Prior to upgrading sass I was getting this error when building with the dynamic-font function:

Screenshot 2023-08-22 at 4 04 52 PM

I found this Stack Overflow thread which mentions upgrading to 1.33.0 or higher and that fixed the problem so this change is required for the build to work.

@brandyscarney
brandyscarney marked this pull request as ready for review August 22, 2023 21:48
@brandyscarney
brandyscarney requested review from a team and thetaPC and removed request for a team August 22, 2023 21:48

@thetaPC thetaPC left a comment

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.

LGTM, just let the team know that they'll need a npm install to avoid the error.

@brandyscarney
brandyscarney merged commit b1cc988 into FW-4146 Aug 24, 2023
@brandyscarney
brandyscarney deleted the FW-4928 branch August 24, 2023 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package: core @ionic/core package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants