Skip to content
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

Add ability to wait for fonts to load with GoogleFonts.pendingFonts #195

Merged
merged 18 commits into from Jun 6, 2023

Conversation

johnsonmh
Copy link
Contributor

@johnsonmh johnsonmh commented Nov 15, 2021

Description

GoogleFonts.pendingFonts returns a Future which resolves when requested fonts have finished loading and are ready to be rendered on screen.

This can be used in combination with a FutureBuilder to avoid visual font swapping

This PR includes these changes:

  • updated example to Material 3
  • showcase using FutureBuilder in example
  • added information to README
  • improved documentation

Related Issues

Tests

  • Added a test

Fixes #151

Future work

  • Consider adding more fine grained control over this feature by providing preloadX functions for each text style and text theme. Currently it is not possible to tell which font loads when.

@@ -99,7 +107,9 @@ TextStyle googleFontsTextStyle({
file: fonts[matchedVariant]!,
);

loadFontIfNecessary(descriptor);
final loadingFuture = loadFontIfNecessary(descriptor);
Copy link
Collaborator

Choose a reason for hiding this comment

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

test cases would be nice to validate what this behaves like when loading the font isn't necessary, for instance

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this has been covered by load_font_if_necessary_with_local_fonts_test.dart.

Copy link
Collaborator

@guidezpl guidezpl left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM as an initial functionality. Brain is a bit foggy, but can't think of a way to avoid pendingFontLoadsInternal

@clocksmith
Copy link
Contributor

instead of return a Future of List of fonts, can this return a List of Future fonts? Also, consider renaming to `loadedFonts, since they aren't necessarily pending when this is called

@devj3ns
Copy link

devj3ns commented Feb 4, 2022

Hey @johnsonmh and @guidezpl, I would really appreciate if someone had the time to finish this PR in the near future.

It's an awesome feature, and you are doing amazing work 💙😄

@therealjbox

This comment was marked as off-topic.

@clocksmith
Copy link
Contributor

@johnsonmh is this still valid?

@lpongetti
Copy link

heyla! it's approved!! pls merge it

Copy link
Collaborator

@guidezpl guidezpl left a comment

Choose a reason for hiding this comment

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

Agree with changes proposed by @clocksmith and testing is still required before merging

@esouthren
Copy link
Contributor

Hey @guidezpl, can you take another pass at this when you get a chance? Thanks!

@guidezpl guidezpl marked this pull request as draft February 3, 2023 10:14
@guidezpl guidezpl closed this Feb 3, 2023
@guidezpl guidezpl deleted the proposal-awaitFontLoads branch February 3, 2023 15:23
@guidezpl guidezpl restored the proposal-awaitFontLoads branch February 3, 2023 15:24
@guidezpl guidezpl reopened this Feb 3, 2023
@arthurbcd
Copy link

It's been 2 years. What happened? Can we merge it? 😄

Copy link
Collaborator

@guidezpl guidezpl left a comment

Choose a reason for hiding this comment

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

I've taken over the PR and made improvements, approving to unblock landing.

@guidezpl guidezpl marked this pull request as ready for review June 5, 2023 12:23
@guidezpl guidezpl requested a review from a team as a code owner June 5, 2023 12:23
@guidezpl guidezpl changed the title [Proposal] Add function to await any pending font loads. Add ability to wait for fonts to load with GoogleFonts.pendingFonts Jun 5, 2023
Copy link
Contributor

@JoseAlba JoseAlba left a comment

Choose a reason for hiding this comment

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

LGTM

@guidezpl guidezpl merged commit 423aea0 into main Jun 6, 2023
7 checks passed
@guidezpl guidezpl deleted the proposal-awaitFontLoads branch June 6, 2023 16:42
@noahjoshua
Copy link

Generally how long does it take for a merge like this to be available in the package?

@guidezpl
Copy link
Collaborator

guidezpl commented Jun 7, 2023

I'm going to fix #102 and then release :)

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.

Add ability to preload fonts