[macOS] Update template icons to Big Sur style #103739
Conversation
cbracken
added
platform-mac
flutter-dashboard
bot
added
the
tool
There was a problem hiding this comment.
Looking at other icons in my dock that have the "logo on a blank blob" style (Chrome, VS Code, Discord), the padding here feels much higher. I would make the logo bigger to look more like those.
But it's also a placeholder, so I don't care that much. LGTM either way :)
stuartmorgan-g
left a comment
stuartmorgan-g
left a comment
There was a problem hiding this comment.
Wait, sorry, this shouldn't be in flutter/flutter. macOS must have gone in before the rules change.
You should put these in https://github.com/flutter/packages/tree/main/packages/flutter_template_images and update the macOS template to pull from there, like Windows does.
|
Yep -- ended up asking @christopherfujino because I swore we'd created a better solution but couldn't find any reference to it on the wiki. I sent out flutter/packages#1930 and will update this patch to a pubspec change and icon removal once that lands, then add details to the wiki. |
|
Re: icon size, this was my basis for comparison: |
|
I guess there's just no real consistency. 🤷🏻 |
cbracken
force-pushed
the
update-macos-icon
branch
from
24c75f6 to
eb88895
Compare
|
@stuartmorgan flutter/packages#1930 has landed and was published as flutter_template_images 4.1.0. I've updated this patch to:
PTAL |
cbracken
force-pushed
the
update-macos-icon
branch
from
eb88895 to
b11c593
Compare
flutter-dashboard
bot
added
the
c: contributor-productivity
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
test-exempt: rolling dependencies |
cbracken
force-pushed
the
update-macos-icon
branch
from
b11c593 to
e7de74b
Compare
stuartmorgan-g
left a comment
stuartmorgan-g
left a comment
There was a problem hiding this comment.
LGTM
Do we know why charcode is moving around in some pubspecs?
I ran a full I may actually send a separate roll for everything but |
|
I wonder if there's an "achievement unlocked" badge you can get for being broken by a recent parameter deprecation in a package that you wrote a decade ago (package:coverage) causing breakage in code that you wrote somewhere else a half-decade ago (coverage in the flutter tool). No good deed goes unpunished... |
This rolls depdendencies to latest using flutter update-packages --force-upgrade This change includes three code changes: * Removes charcode from the dependencies allowlist since it no longer appears in the transitive closure of dependencies of the flutter, flutter_test, flutter_driver, flutter_localizations, and integration_test packages. * Uses Resolver.create instead of the deprecated Resolver constructor. The default Resolver constructor has been deprecated in favour of the static Resolver.create() factory function, which unfortunately happens to be async. Propagated the async-ness up the chain. * Eliminates the use of the deprecated packagesPath parameter to HitMap.parseJson. This parameter was deprecated and replaced with packagePath in dart-archive/coverage#370 which was part of the overall deprecation of the .packages file in Dart itself dart-lang/sdk#48272. The overall goal being that end-user code shouldn't need to know about implementation details such as whether dependency information is stored in a .packages file or a package_info.json file, but rather use the package_config package to obtain the package metadata and perform other functions such as resolving its dependencies to filesystem paths. packagesPath was replaced by packagePath, which takes the path to the package directory itself. Internally, package:coverage then uses package_config to do the rest of the package/script URI resolution to filesystem paths. This is a pre-update prior to updating flutter_template_images in flutter#103739 Issue: flutter#103371 Issue: flutter#103775
Roll dependendencies This rolls depdendencies to latest using flutter update-packages --force-upgrade This change includes three code changes: * Removes charcode from the dependencies allowlist since it no longer appears in the transitive closure of dependencies of the flutter, flutter_test, flutter_driver, flutter_localizations, and integration_test packages. * Uses Resolver.create instead of the deprecated Resolver constructor. The default Resolver constructor has been deprecated in favour of the static Resolver.create() factory function, which unfortunately happens to be async. Propagated the async-ness up the chain. This change was partially reverted and the deprecation ignored in this patch until package:coverage can be rolled internally at Google. * Eliminates the use of the deprecated packagesPath parameter to HitMap.parseJson. This parameter was deprecated and replaced with packagePath in dart-archive/coverage#370 which was part of the overall deprecation of the .packages file in Dart itself dart-lang/sdk#48272. The overall goal being that end-user code shouldn't need to know about implementation details such as whether dependency information is stored in a .packages file or a package_info.json file, but rather use the package_config package to obtain the package metadata and perform other functions such as resolving its dependencies to filesystem paths. packagesPath was replaced by packagePath, which takes the path to the package directory itself. Internally, package:coverage then uses package_config to do the rest of the package/script URI resolution to filesystem paths. This change was partially reverted and the deprecation ignored in this patch until package:coverage can be rolled internally at Google. This is a pre-update prior to updating flutter_template_images in #103739 Issue: #103371 Issue: #103775 Issue: #103830 When re-applying the partially-reverted changes to code coverage, we'll need to patch host_entrypoint.dart internally to await the Future that we'll be returning rather than a non-async value.
Updates the AppIcon assets in the macOS app template to meet Apple's icon guidelines as of macOS Big Sur. Assets were moved to the [flutter_template_images][1] package, which includes them as of version 4.1.0. Flutter packages were upgraded. `charcode` no longer occurs in the transitive closure of dependencies and thus was removed from the allowlist in dev/bots/allowlist.dart. Ref: https://developer.apple.com/design/human-interface-guidelines/macos/icons-and-images/app-icon/ Issue: flutter#103371 [1]: https://github.com/flutter/packages/tree/main/packages/flutter_template_images
cbracken
force-pushed
the
update-macos-icon
branch
from
e7de74b to
88e82b6
Compare
3 participants
Updates the AppIcon assets in the macOS app template to meet Apple's
icon guidelines as of macOS Big Sur. Assets were moved to the
flutter_template_images package, which includes them as of version
4.1.0.
Ref: https://developer.apple.com/design/human-interface-guidelines/macos/icons-and-images/app-icon/
Issue: #103371
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.