Skip to content

[flutter_template_images] Add support for new template and shared platform files #295

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.

Sign up for GitHub

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

Jump to bottom
Merged
stuartmorgan-g merged 5 commits into from
Mar 10, 2021
Merged

[flutter_template_images] Add support for new template and shared platform files #295

stuartmorgan-g merged 5 commits into from
Mar 10, 2021

Conversation

@brianegan
Copy link
Contributor

@brianegan brianegan commented Mar 1, 2021

There is a proposal to add a new Flutter app template to the flutter CLI. This new app template lives alongside the current counter template, it does not replace it. The team has made the following request on the related PR:

We need to find a way to not duplicate the existing platform files. We occasionally have to update these, and having 2 copies of everything is going to increase the chances that we accidentally get out of sync somewhere.

This PR moves assets common to both apps into an app_shared directory and introduces a new list_detail_app folder to support images unique to the new template.

Related Issues:

Pre-launch Checklist

  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
  • I updated CHANGELOG.md to add a description of the change.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

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

@brianegan brianegan mentioned this pull request Mar 1, 2021
Closed
8 tasks
@stuartmorgan-g stuartmorgan-g self-requested a review March 3, 2021 21:05
@brianegan brianegan force-pushed the new_template_images branch from 9191b27 to 3ebe2a7 Compare March 4, 2021 10:31
@brianegan brianegan force-pushed the new_template_images branch from 3ebe2a7 to a82a6e0 Compare March 4, 2021 13:15
stuartmorgan-g
stuartmorgan-g requested changes Mar 4, 2021
View reviewed changes
Copy link
Collaborator

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

If you want to use this in flutter/flutter it needs to be published, so this must include a version change and corresponding CHANGELOG entry.

goderbauer
goderbauer reviewed Mar 4, 2021
View reviewed changes
Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

Can you bump the version in the pubspec and update the changelog? Thanks!

@goderbauer
Copy link
Member

goderbauer commented Mar 4, 2021

Oh, sorry, didn't see that @stuartmorgan already said that above... :)

@brianegan
Copy link
Contributor Author

brianegan commented Mar 5, 2021 •
edited
Loading

Thanks so much, @stuartmorgan and @goderbauer :) I've updated the PR to bump the CHANGELOG and pubspec.

stuartmorgan-g
stuartmorgan-g approved these changes Mar 8, 2021
View reviewed changes
Copy link
Collaborator

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

LGTM assuming the flutter/flutter PR is approved with this structure. I'd like to wait on landing this until that's approved just in case there are any changes.

@brianegan
Copy link
Contributor Author

brianegan commented Mar 10, 2021

Thanks @stuartmorgan! I have gotten a "LGTM" on the Template. Is that enough to land this PR and publish it up?

I'm in a bit of a catch-22 here, haha. In order for the all my tests to pass on the Flutter PR, this new version needs to be deployed to pub :)

@stuartmorgan-g
Copy link
Collaborator

stuartmorgan-g commented Mar 10, 2021

Yes, that's all it needed. I realized just as I was about to land it that the version bump needs to be to 2.0 since it's a breaking change though (moving existing files), so I've changed that. Once the tests re-run and are green I'll land and publish.

@stuartmorgan-g stuartmorgan-g merged commit 878688c into flutter:master Mar 10, 2021
@stuartmorgan-g
Copy link
Collaborator

stuartmorgan-g commented Mar 10, 2021

Published 2.0.0

@brianegan brianegan deleted the new_template_images branch March 10, 2021 17:26
@brianegan
Copy link
Contributor Author

brianegan commented Mar 10, 2021

Excellent, thanks for your help @stuartmorgan :)

stuartmorgan-g pushed a commit to stuartmorgan-g/packages that referenced this pull request Apr 30, 2021
@ammaratef45
add empty text child for a if [](https://...) is provided (flutter#295)
cb4a890
stuartmorgan-g pushed a commit that referenced this pull request May 13, 2021
@stuartmorgan
Revert "[flutter_template_images] Add support for new template and sh…
f8638c0 
…ared platform files (#295)"

This reverts commit 878688c.
@stuartmorgan-g stuartmorgan-g mentioned this pull request May 13, 2021
Merged
stuartmorgan-g added a commit that referenced this pull request May 14, 2021
@stuartmorgan-g
Revert "[flutter_template_images] Add support for new template and sh…
6081b46 
…ared platform files" (#353)

Reverts #295

The corresponding flutter/flutter changes never landed, so that package layout is unusable for now. Since 2.0 was published already, to minimize long-term confusion this uses 3.0 rather than going back to 1.0.
@filiph filiph mentioned this pull request May 28, 2021
Merged
11 tasks
austinstoker pushed a commit to austinstoker/packages that referenced this pull request Apr 29, 2022
@slovnicki
improve Android back button behavior
d8698c8 
        by changing a bit the implementation of BeamerBackButtonDispatcher and adding
        BeamerChildBackButtonDispatcher that can be automatically created in nested Beamers.
        Dispatcher now also has an optional onBack closure for custom back button handling.

        This also makes BeamerDelegate.active public so it can be used in those edge cases where it's needed.

        - closes flutter#279
	- closes flutter#295
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@goderbauer goderbauer goderbauer left review comments

@stuartmorgan-g stuartmorgan-g stuartmorgan-g approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@brianegan @goderbauer @stuartmorgan-g