-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Add Linux and Windows target platforms #51519
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
Conversation
60ec054 to
11bdcf0
Compare
|
/cc @stuartmorgan |
stuartmorgan-g
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this actually what we want, given that Roboto isn't on Windows or Linux by default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I thought about that, but it's the default font for Material, and we're doing Material on these platforms. I think it might look wrong to, say, use "Arial" on Windows and, "San Francisco" on macOS. We can try it, though, if that makes it a smaller download. Don't we package all the fonts in use?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's the default font for Material
Is it though?
"Roboto is the default system font for Android. For platforms outside of Android and web products, use a system typeface that is preferred on that platform. For example, iOS applications should use Apple’s San Francisco font."
https://material.io/design/typography/understanding-typography.html#system-fonts
Don't we package all the fonts in use?
It's possible to package fonts in an application, but it's not done by default that I'm aware of. Doing it yourself via pubspec doesn't have a per-platform specification that I'm aware of, so I don't think it's possible to have an app that packages Roboto for Windows and Linux without bloating everything else (which would be really bad for iOS and Android), although maybe I'm wrong about that.
Also, if we wanted to auto-package it, then we would need to add tool support to auto-download it, since we can't package something that's not on the machine to start with.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh, I was mistaken about that then. I'll switch it to default to "Segoe UI" on Windows. On Linux, it's not so clear: Ubuntu's default font is "Ubuntu", so I'm guessing that it's not available on Embedded Gentoo. :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also probably find out if you can specify fonts per platform in a pubspec: someone's definitely going to want that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should talk to folks about how to handle Linux. We may need a way to specify a list of a few options depending on what's present, or we may need to pick one that we think is most likely and then rely on font fallback (which is currently awful on Linux per recent discussions, but needs to be fixed anyway).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I added whiteRedmond and blackRedmond typography sets based on "Segoe UI". I left the linux as "Roboto", for lack of a better default for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I made linux use Roboto, but added fallbacks of DejaVu Sans, Liberation Sans, and finally Arial.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still not sure about Roboto as the first choice, but we can revisit that later; this is clearly better than nothing. Speaking of revisiting though, will we consider changing this list later to be a breaking change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a tough one. One one hand, rendering run on the same machine might change, on the other, the presence of a particular font is purely a stochastic sample.
I'd lean towards "yes", simply because it might change goldens for some testers.
|
This isn't a breaking change? |
|
Hmm. I guess it could cause new warnings about unhandled switch cases (if that lint is turned on). OK, I'll go with "breaking change". |
21d1b89 to
3bf8df6
Compare
fe18b1a to
7dc69c5
Compare
Remove a target platform override that is no longer needed (I missed this in #51519).
Description
This PR adds the
linuxandwindowstarget platform enum values, along with automatically setting thedefaultTargetPlatformto the appropriate value on those platforms.Related Issues
Tests
Breaking Change