Skip to content
This repository was archived by the owner on Jun 13, 2024. It is now read-only.

Update dependencies and platform directories #325

Merged
merged 4 commits into from
Sep 23, 2020
Merged

Update dependencies and platform directories #325

merged 4 commits into from
Sep 23, 2020

Conversation

gspencergoog
Copy link
Contributor

@gspencergoog gspencergoog commented Sep 23, 2020 •
edited
Loading

Description

This updates the url_launcher to ^5.6.0 so that it includes Windows, and adds the missing platform directories for web, windows, and linux.

Related Issues

@gspencergoog gspencergoog mentioned this pull request Sep 23, 2020
Closed
@gspencergoog gspencergoog force-pushed the update_dependencies branch 3 times, most recently from 7f3ea2b to 879287c Compare September 23, 2020 13:12
@stuartmorgan-g
Copy link
Contributor

stuartmorgan-g commented Sep 23, 2020 •
edited
Loading

adds the missing platform directories for web, windows, and linux

There's already a web directory on trunk. I would suggest reverting all the ios/, android/, macos/, and web/ changes in this PR so that it's scoped specifically to adding Linux and Windows support, and doesn't have unintended side-effects on other platforms.

rami-a
rami-a reviewed Sep 23, 2020
View reviewed changes
Copy link
Contributor

@rami-a rami-a left a comment

Choose a reason for hiding this comment

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

I think adding the windows/linux directories makes sense. But it looks like this change is also modifying android/iOS/macOS/web in ways we don't necessarily want, such as removing the custom app icons

android/app/src/main/res/mipmap-anydpi-v26/ic_launcher.xml Outdated
@@ -1,5 +0,0 @@
<?xml version="1.0" encoding="utf-8"?>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are there all of these Android changes? Also this looks like it is removing our Android adaptive icon. I don't think we wanna do that.

ios/Podfile Outdated
@@ -1,41 +0,0 @@
# Uncomment this line to define a global platform for your project
Copy link
Contributor

Choose a reason for hiding this comment

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

Same questions as above, why does this change delete a bunch of iOS related files?

macos/Podfile Outdated
@@ -1,82 +0,0 @@
platform :osx, '10.11'
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we be deleting these macOS files?

macos/Runner.xcodeproj/project.pbxproj Outdated
@@ -28,7 +28,6 @@
33CC11132044BFA00003C045 /* MainFlutterWindow.swift in Sources */ = {isa = PBXBuildFile; fileRef = 33CC11122044BFA00003C045 /* MainFlutterWindow.swift */; };
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like you deleted the gallery macOS app icon and replaced it with the default flutter icon. Github wouldn't let me comment on the image so I commented here. I don't think we want to do that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I already reverted that. Thanks for pointing it out.

web/index.html Outdated
@@ -12,15 +12,9 @@
<link rel="apple-touch-icon" href="icons/Icon-192.png">

<!-- Favicon -->
<link rel="icon" type="image/png" href="favicon-16x16.png" sizes="16x16"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

This change also removed our custom gallery favicon

@stuartmorgan-g
Copy link
Contributor

Is there a definitive source for the Gallery icon? At some point (probably not in this PR) the default .ico in the Windows folder should be replaced with one for Gallery.

@gspencergoog gspencergoog force-pushed the update_dependencies branch 2 times, most recently from 8ed3171 to 5d4e5ca Compare September 23, 2020 13:41
@rami-a
Copy link
Contributor

rami-a commented Sep 23, 2020 •
edited
Loading

Is there a definitive source for the Gallery icon? At some point (probably not in this PR) the default .ico in the Windows folder should be replaced with one for Gallery.

Yes, I found the original sketch file that the designers created and shared it with the flutter team group. Also the highest res iOS asset could be used as a source of truth as it is 1024x1024

https://github.com/flutter/gallery/blob/master/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-1024x1024%401x.png

@gspencergoog
Copy link
Contributor Author

I actually already reverted most of the macos/ios/android changes. Sorry, I was a little hasty in posting the PR, and I hadn't fully checked it on all the platforms yet. I was mainly creating it to see if the checks would pass, and I didn't think anyone would look at it at 6am PST. :-)

I did want to update the versions (kotlin, gradle, min OS version, etc.) in the macos/ios/android directories, but I can do that in another PR.

@gspencergoog
Copy link
Contributor Author

I did update the default windows icon though: we already had an .ico file for it in the tree.

@stuartmorgan-g
Copy link
Contributor

I did update the default windows icon though: we already had an .ico file for it in the tree.

It looks like it's a web favicon that only has 16x16, unless GitHub's UI isn't giving me the right bits. The default Windows icon has 256, 48, 32, and 16, which based on some research at the time seemed to be the right set to cover for Win32 apps.

@gspencergoog
Copy link
Contributor Author

OK, I updated it for those sizes.

rami-a
rami-a approved these changes Sep 23, 2020
View reviewed changes
Copy link
Contributor

@rami-a rami-a left a comment

Choose a reason for hiding this comment

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

LGTM

@gspencergoog
Copy link
Contributor Author

Actually, I'm going to make a better version: the small sizes in that .ico are not great.

@gspencergoog gspencergoog merged commit 14ad507 into flutter:master Sep 23, 2020
@gspencergoog gspencergoog deleted the update_dependencies branch September 23, 2020 15:08
@rami-a rami-a mentioned this pull request Sep 24, 2020
Closed
Piinks pushed a commit to Piinks/gallery that referenced this pull request Oct 12, 2020
@prasadsunny1
Update cupertino_icons: from 0.1.2 to 0.1.3 (flutter#325)
b871178 
Former-commit-id: d04d40d
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@rami-a rami-a rami-a 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.

Flutter Gallery release for Windows
3 participants
@gspencergoog @stuartmorgan-g @rami-a