-
Notifications
You must be signed in to change notification settings - Fork 29
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
fix: added fixes for flutter 3.22 #32
Conversation
@ali-thowfeek Bumping this, as our project depends on it as well |
I really need this fix too! |
I need this PR to be merged and released too! 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@erickgon92 Thank you for the PR. Really appreciate the time you put in 😇. There are few things we need to get right before we can go ahead with this PR.
Font sizes reference:
https://github.com/flutter/flutter/blob/5dcb86f68f239346676ceb1ed1ea385bd215fba1/packages/flutter/lib/src/material/text_theme.dart
@@ -161,7 +160,7 @@ class Parser { | |||
case 'h1': | |||
double h1; | |||
if (defaultFontSize == null) { | |||
h1 = Theme.of(context).textTheme.headline5?.fontSize ?? 24.0; | |||
h1 = Theme.of(context).textTheme.headlineLarge?.fontSize ?? 24.0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be headlineSmall
@@ -171,7 +170,7 @@ class Parser { | |||
case 'h2': | |||
double h2; | |||
if (defaultFontSize == null) { | |||
h2 = Theme.of(context).textTheme.headline6?.fontSize ?? 20.0; | |||
h2 = Theme.of(context).textTheme.headlineLarge?.fontSize ?? 20.0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be titleLarge
@@ -181,7 +180,7 @@ class Parser { | |||
case 'h3': | |||
double h3; | |||
if (defaultFontSize == null) { | |||
h3 = Theme.of(context).textTheme.subtitle1?.fontSize ?? 16.0; | |||
h3 = Theme.of(context).textTheme.headlineMedium?.fontSize ?? 16.0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be bodyLarge
@@ -191,7 +190,7 @@ class Parser { | |||
case 'h4': | |||
double h4; | |||
if (defaultFontSize == null) { | |||
h4 = Theme.of(context).textTheme.bodyText1?.fontSize ?? 16.0; | |||
h4 = Theme.of(context).textTheme.headlineMedium?.fontSize ?? 16.0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be bodyLarge
@@ -201,7 +200,7 @@ class Parser { | |||
case 'h5': | |||
double h5; | |||
if (defaultFontSize == null) { | |||
h5 = Theme.of(context).textTheme.bodyText1?.fontSize ?? 16.0; | |||
h5 = Theme.of(context).textTheme.headlineMedium?.fontSize ?? 16.0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be bodyLarge
@@ -211,7 +210,7 @@ class Parser { | |||
case 'h6': | |||
double h6; | |||
if (defaultFontSize == null) { | |||
h6 = Theme.of(context).textTheme.bodyText2?.fontSize ?? 14.0; | |||
h6 = Theme.of(context).textTheme.headlineSmall?.fontSize ?? 14.0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be bodyMedium
Thank you all for your time!! I will make the fixes and inform you. Cheers all! |
@erickgon92 I've already merged your PR. Made the necessary changes on top myself. So no more changes needed on your part. New version 5.0.0 released. Thank you once again. |
Updated libraries and fixed all necessary lines where the deprecated elements where being used.