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

fix: add package parameter to generated asset provider method #361

Merged

Conversation

orevial
Copy link
Contributor

@orevial orevial commented Feb 22, 2023

Closes #360

What does this change?

Make generated Image.provider method accepts an optional package name, which default to current package when using package_parameter_enabled flag

Fixes #360 🎯

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open a GitHub issue as a bug/feature request before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
    • Ensure the tests (melos run unit:test)
    • Ensure the analyzer and formatter pass (melos run format to automatically apply formatting)
  • Appropriate docs were updated (if necessary)

@orevial orevial requested a review from wasabeef as a code owner February 22, 2023 09:41
@@ -570,7 +570,16 @@ class AssetGenImage {
);
}

ImageProvider provider() => AssetImage(_assetName);
ImageProvider provider({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the one and only real change :

  • provider method now sets the package if the flag is enabled
  • provider method now accepts arguments that mirror the AssetImage constructor

Note that the rest of the files are only updates of tests and examples

@wasabeef wasabeef assigned wasabeef and unassigned wasabeef Feb 22, 2023
@codecov
Copy link

codecov bot commented Feb 22, 2023

Codecov Report

Merging #361 (b077014) into main (ef7ba1f) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #361   +/-   ##
=======================================
  Coverage   98.26%   98.26%           
=======================================
  Files          20       20           
  Lines         693      693           
=======================================
  Hits          681      681           
  Misses         12       12           
Impacted Files Coverage Δ
packages/core/lib/generators/assets_generator.dart 98.09% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@wasabeef wasabeef merged commit b559901 into FlutterGen:main Apr 17, 2023
@wasabeef
Copy link
Member

@orevial
Thank you. I plan to release a new version today.

@blaugold
Copy link
Contributor

blaugold commented Apr 17, 2023

Thanks @orevial and @wasabeef!

@wasabeef wasabeef mentioned this pull request Apr 18, 2023
@wasabeef wasabeef added this to the 5.3.0 milestone Apr 18, 2023
@wasabeef
Copy link
Member

@orevial @blaugold @spilioio

I just released v5.3.0. Sorry I kept you waiting.

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.

[BUG]: package_parameter_enabled is ignored for AssetGenImage "provider" method
4 participants