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 asset transformation sample #2267

Merged
merged 14 commits into from May 14, 2024
Merged

Add asset transformation sample #2267

merged 14 commits into from May 14, 2024

Conversation

andrewkolos
Copy link
Contributor

@andrewkolos andrewkolos commented Apr 30, 2024

This adds a sample Flutter project that demonstrates a soon-to-be-released feature, asset transformation1. PR for flutter.dev documentation.

This feature isn't the easiest to explain using documentation, so I think augmenting that documentation with a sample is appropriate.

This sample demonstrates 1) how to use an existing Dart package (that is compatible with the feature) as an asset transformer and 2) how to write a Dart package that is compatible with this feature. This should be clear from the README.md.

Advice for reviewing this PR. The goal here is that most users that read the documentation and follow the link from there to this sample should be able to figure out what the feature does and how to use it. Try to imagine yourself in this position and follow this story. If the feature is still unclear, then there is probably something we can do to improve this sample or the docs. Said more simply, follow these steps: 1) Start at the new section to be added to Flutter.dev (https://flutter-docs-prod--pr10471-document-asset-transformers-cc21qf01.web.app/ui/assets/assets-and-images#automatic-transformation-of-asset-files-at-build-time). It should naturally link you to the sample project. Start with the README and see if things make sense.

Pre-launch Checklist

  • I read the Flutter Style Guide recently, and have followed its advice.
  • I signed the CLA.
  • I read the Contributors Guide.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-devrel channel on Discord.

Footnotes

  1. If you are super curious about this feature, see the tracking issue for its implementation.

@domesticmouse
Copy link
Contributor

@andrewkolos
Copy link
Contributor Author

andrewkolos commented Apr 30, 2024

qq: which version of Flutter will this land in?

It's already landed in master, so the upcoming stable release—3.22.

@andrewkolos andrewkolos changed the title [WIP] Add asset transformation sample Add asset transformation sample May 1, 2024
@andrewkolos andrewkolos marked this pull request as ready for review May 1, 2024 00:45
@domesticmouse
Copy link
Contributor

qq: which version of Flutter will this land in?

It's already landed in master, so the upcoming stable release—3.22.

If that's the case, I'd skip experimental and make it a top level sample. We can land this PR when 3.22 becomes stable.

@andrewkolos
Copy link
Contributor Author

qq: which version of Flutter will this land in?

It's already landed in master, so the upcoming stable release—3.22.

If that's the case, I'd skip experimental and make it a top level sample. We can land this PR when 3.22 becomes stable.

Makes sense. Done.

Copy link
Contributor

@domesticmouse domesticmouse left a comment

Choose a reason for hiding this comment

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

LGTM.

Land when Flutter 3.22 goes stable.

@domesticmouse
Copy link
Contributor

Actually, I'm confused. CI is showing stable channel as green. Shouldn't CI show stable channel as red?

@andrewkolos
Copy link
Contributor Author

Actually, I'm confused. CI is showing stable channel as green. Shouldn't CI show stable channel as red?

The sample didn't include a test, so I just added one.

sfshaza2 added a commit to flutter/website that referenced this pull request May 12, 2024
Documents the new asset transformation feature (if curious, see
flutter/flutter#143348; but the documentation
included in this PR should be sufficient for learning about and
understanding this feature.)

**For reviewers.** The documentation includes a link to a
yet-to-be-merged sample in the flutter/samples repo. When you arrive at
this link, I ask that you review
flutter/samples#2267, which is the PR that adds
the sample being linked to.

## Presubmit checklist

- [X] This PR doesn’t contain automatically generated corrections
(Grammarly or similar).
- [X] This PR follows the [Google Developer Documentation Style
Guidelines](https://developers.google.com/style) — for example, it
doesn’t use _i.e._ or _e.g._, and it avoids _I_ and _we_ (first person).
- [X] This PR uses [semantic line
breaks](https://github.com/dart-lang/site-shared/blob/main/doc/writing-for-dart-and-flutter-websites.md#semantic-line-breaks)
of 80 characters or fewer.

---------

Co-authored-by: Brett Morgan <[email protected]>
Co-authored-by: Shams Zakhour (ignore Sfshaza) <[email protected]>
@domesticmouse
Copy link
Contributor

@andrewkolos can you please update this PR? It is 20 PRs behind main.

@domesticmouse
Copy link
Contributor

This is probably going to have to wait for the fixes in #2266 before the CI will turn green

@ericwindmill ericwindmill merged commit 821422f into main May 14, 2024
13 of 14 checks passed
@ericwindmill ericwindmill deleted the asset-transformation branch May 14, 2024 15:42
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.

None yet

3 participants