ionic-team / ionic-framework

A powerful cross-platform UI toolkit for building native-quality iOS, Android, and Progressive Web Apps with HTML, CSS, and JavaScript.
https://ionicframework.com
MIT License
51.13k stars 13.5k forks source link

v4: inequality on font between v3 and v4 #14707

Closed mburger81 closed 5 years ago

mburger81 commented 6 years ago

Describe the Bug If you have a look on the two screenshots a attached and have a look on the text there you can see they looks quite different between v3 and v4. For example on the LANTHINGS header you can see the difference but if you see the computed style values in DevTools anything should be the same, same font-family, font-size and font-weight.

We are not an expert on MD so we are not sure if the text looks different from v3 to v4 because MD in meantime has changed or if there is some but in v4 or perhaps on v3, if so please forgive me!

By the way we think text in v3 looks better :smiley:

V3: image

V4: image

manucorporat commented 6 years ago

I agree the title looks different, can you do some research about what actually changed? in the computed styles i see no difference...

mburger81 commented 6 years ago

I'll try to do some research later, but can you see the same behavior on your side, comparing ionic-conference-app between v3 and v4, we can see this difference on schdedule page

For sure the Menu header is different and also the TAB titles ALL and FAVORITES are different. Not sure about the menu and list font, it looks a little bit different but we could be wrong.

v3 image

v4 image

jgw96 commented 6 years ago

Hello @mburger81 ! Is this still an issue with alpha.10?

mburger81 commented 6 years ago

@jgw96 No as you can see in the screenshots the problem is not resolved. I update your ionic-conference-app with latest alpha.10 version. The fonts in Toolbar and also every where else in the page looks quite different. Also something we have noticed in our migration, the app looks generally different and mainly because many margins/padding and sizes seems to have changed.

v3 image v4 image

mburger81 commented 6 years ago

Is there a progress on this issue? on beta.6 there is still the same issue

brandyscarney commented 6 years ago

@mburger81 I'm not able to reproduce this in Google Chrome, Firefox, or Safari. May I ask what operating system and browser you are using? Sorry if this was stated already somewhere else.

You could also try this extension to compare the CSS between the two: https://chrome.google.com/webstore/detail/css-diff/pefnhibkhcfooofgmgoipfpcojnhhljm?hl=en

mburger81 commented 6 years ago

@brandyscarney We use on all our dev pcs (k)ubuntu with always latest chrome. We compared the css and both seems identically on the font styles no diff. But the text on browser looks completely different.

If can rember well we tested it also on a windows with the same result. But not sure abaut it.

I can use the Extension Monday to compare the css better! Can you add some screenshots?

brandyscarney commented 6 years ago

Chrome

screen shot 2018-08-30 at 2 07 50 pm

Firefox

screen shot 2018-08-30 at 2 09 20 pm

I can see what you're saying with the margin/padding between icons & text. This seems to be this bug here: https://github.com/ionic-team/ionic/issues/14799

mburger81 commented 6 years ago

@brandyscarney I'm comparing the ion-title in an ion-toolbar as you can see in the screenshot it looks completely different. v3 image v4 image

I used CSS DIFF to compare the SLOT of ion-title in ionic4 and the DIV in ion-title in ionic3 and as you can see there is mainly no difference. image

Another example is, the PUBLIC text you can see in the screenshots this is the code we have

<ion-content id="content" padding class="main-content">
    <ion-grid no-padding>
        <ion-row *showItBootstrap="['md', 'lg', 'xl']">
            <ion-col>
                <p text-uppercase>{{ title }}</p>
            </ion-col>
        </ion-row>
        <ion-row>

So as you can see nothing magical, but this is the DIFF image different font-size, different height and different margin in v3 the font-size is coming from ion-app.md with font-size: 1.4rem in v4 font-size is not set at all the margin seems to be for both versions

p {
    display: block;
    -webkit-margin-before: 1em;
    -webkit-margin-after: 1em;
    -webkit-margin-start: 0px;
    -webkit-margin-end: 0px;
}

so as we know em is based on the width of the uppercase M, this means if the margin is different the FONT between the two apps is also different. Right?

Can we check if the used font is really "Roboto" instead of "Helvetica Neue" or "sans-serif"? I think the problem is the loaded font, or not?

If we do this document.fonts.check('1em Roboto'); in console in v3 we got TRUE as result in v4 we got FALSE as result So the problem should be the wrong loaded font family?

Edit: Sorry for the question, but what is the default Font-Family in MD for Ionic? Roboto? Right? In v3 we can see a network traffic for downloading roboto-regular.woff2 and roboto-medium.woff2 which we does not have for v4.

mburger81 commented 6 years ago

@brandyscarney @manucorporat in v3 we have in the variable.scss the @import "roboto"; and in src/fontswe have all roboto and sans-serif fonts. Where is this on v4?

In the core.scss we can see that we load the default font for md

html.md {
  --ion-default-font: "Roboto", "Helvetica Neue", sans-serif;
}

Which should be always Robot. But the font is present neither in the core project nor in the starter template? And perhaps I'm missing something but I can't remember that the font should be included in browser or OS by default? Am I the only one who has this problem? I think if we can resolve this most of the spacing and margin issues which are working on "em" should be resolved.

P.S. sorry if I should not comment you

mburger81 commented 6 years ago

Today I tested also at home to a W10 pc, same behaivor.

mburger81 commented 6 years ago

@brandyscarney @manucorporat Mike yesterday told me (as we already thought and saw) that you don't include Roboto fonts in v4 any more because you use the default system fonts. Okay on Android 4.1 the Roboto font should be the default sans-sarif font. But if you run the same app on MD using ionic serve or as a web app in any browser on any desktop system or ios, there is no Roboto font loaded at all. Mac, Linux and Windows does not include by default Roboto font neither by the font it self nor as default sans-serif font.

So, IMO you have to include the fonts and load them them if needed. This means if you are on Android > 4.1 and you are using MD mode you can speculate that the sans-serif font-family is the Roboto font and you have nothing to do. But on any other system if you have MD you have to load the fonts in any way.

We saw that you have still the fonts on your github repo, under the app component https://github.com/ionic-team/ionic/tree/master/core/src/components/app/fonts There is also the roboto.scss file which loads the font-face, but this roboto.scss file is nowhere imported.

I'm not an expert on this, but I think: There are several ways to this with different problems.

To have lazy loading you could use FontFace API which does not work on all browsers. https://developers.google.com/web/fundamentals/performance/optimizing-content-efficiency/webfont-optimization?hl=de https://drafts.csswg.org/css-font-loading/#fontface

     if (!platform.is('android') && !document.fonts.check('1em Roboto')) {
            console.log('you are running outside android and Robot font is probably not present')

            const rN300 = new window.FontFace("Roboto", "url(/assets/fonts/roboto-light.woff2)", {
                style: 'normal',
                weight: '300'
            });
            rN300.load().then(function (loadedFace) {
                document.fonts.add(loadedFace);
            });
            const rN400 = new window.FontFace("Roboto", "url(/assets/fonts/roboto-regular.woff2)", {
                style: 'normal',
                weight: '400'
            });
            rN400.load().then(function (loadedFace) {
                document.fonts.add(loadedFace);
            });
            const rN500 = new window.FontFace("Roboto", "url(/assets/fonts/roboto-medium.woff2)", {
                style: 'normal',
                weight: '500'
            });
            rN500.load().then(function (loadedFace) {
                document.fonts.add(loadedFace);
            });
            const rN700 = new window.FontFace("Roboto", "url(/assets/fonts/roboto-boldocument.woff2)", {
                style: 'normal',
                weight: '700'
            });
            rN700.load().then(function (loadedFace) {
                document.fonts.add(loadedFace);
            });
        }

In this case we always load all font-face and not only that one we really need.

We tried to add a class to html if we know to need importing Roboto font style, but nesting scss with font-face seems to not work?

other inputs https://www.filamentgroup.com/lab/font-loading.html https://www.filamentgroup.com/lab/font-events.html

mburger81 commented 6 years ago

In my case lazy loading FontFace is always loading all declared fonts not only that one which are needed. If this is not liked

We can declare the font-face in CSS and adding Roboto to font-family ONLY if we are outside of android and using MD mode. In this case on android we have to use sans-serif which is Roboto

Could be this a solution?

brandyscarney commented 6 years ago

@mburger81 Roboto should be used by default on systems that have it installed, with a fallback to Helvetica Neue if not. Since I have Roboto installed on my laptop, I disabled it for testing purposes and I am seeing the correct font. Here are some screenshots:

Roboto installed - uses Roboto:

screen shot 2018-09-07 at 10 40 12 am

Roboto disabled - uses Helvetica Neue:

screen shot 2018-09-07 at 10 39 09 am

I'm not able to see anything as drastic as what you're seeing, so Mike is going to test it out on a linux computer. Could you possibly try this extension to see what font is being used: https://chrome.google.com/webstore/detail/whatfont/jabopobgcpjmedljpbcaablpmlmfcogm?hl=en

mhartington commented 6 years ago

:wave:

I just loaded up the conference app on my linux machine and it wasn't able to replicate the issue. I will note that this machine does not have Roboto or Helvetica, so it used the fall back system sans-serif font screenshot_20180907_105448

I did not modify any css rules, and the toolbar title looked correct on my end.

mburger81 commented 6 years ago

I'm out of office.

But honestley how could we use MD without Roboto font thats not MDπŸ˜‰

I understand you are always thinking ok MD is for android and nothing more. But thats not real for all uf us. If you would like to have a MD for desktop or for an elctron app which is a big use case for many of us, without having font installed which is not the default for any pc you will expring a completly different look. So the same app for android, looks quite different on any desktop pc.

The only thing I will saying is. This is a big breaking change and I think you could change this behavior. You can dynamically load the Roboto fonts if you are not on android and if they are not installed, you can simple check this.

Using another font means many css styles using line heigt or other looks different. For example using roboto font the text is vertically centered in a button using the default sans-serif font on linux or windows it is not centered. Which is another issue I have opened and not unterstand for a lot od time why that was happening. But now, after knowing this many MD style issues are clear and obsoloet.

IMO thx a lot! 😊😊

Brandy Carney notifications@github.com schrieb am Fr., 7. Sep. 2018, 16:49:

@mburger81 https://github.com/mburger81 Roboto should be used by default on systems that have it installed, with a fallback to Helvetica Neue if not. Since I have Roboto installed on my laptop, I disabled it for testing purposes and I am seeing the correct font. Here are some screenshots:

Roboto installed - uses Roboto:

[image: screen shot 2018-09-07 at 10 40 12 am] https://user-images.githubusercontent.com/6577830/45225639-05110580-b28b-11e8-8aa5-f8926ffd7557.png

Roboto disabled - uses Helvetica Neue:

[image: screen shot 2018-09-07 at 10 39 09 am] https://user-images.githubusercontent.com/6577830/45225638-05110580-b28b-11e8-89e8-7d33ef132e93.png

I'm not able to see anything as drastic as what you're seeing, so Mike is going to test it out on a linux computer. Could you possibly try this extension to see what font is being used: https://chrome.google.com/webstore/detail/whatfont/jabopobgcpjmedljpbcaablpmlmfcogm?hl=en

β€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ionic-team/ionic/issues/14707#issuecomment-419463203, or mute the thread https://github.com/notifications/unsubscribe-auth/ADmpTCfxnufJPaRE21YWdBCcC65Qbhzlks5uYodegaJpZM4VCOK0 .

mburger81 commented 6 years ago

Are you sure this looks like equals to Roboto font? I'm on mobile phone so I'm not able to compare this. This is the default font for sans serif on linux which looks similar to roboto font but it is not the same. Not the same on many side like line height and others. Similar is not equal 😊😊

In this case not adding font means the MD look is random, depending on the system and depending on the default sans serif installed.

Not sure if this is what you would like to have at the end.

Mike Hartington notifications@github.com schrieb am Fr., 7. Sep. 2018, 16:59:

πŸ‘‹

I just loaded up the conference app on my linux machine and it wasn't able to replicate the issue. I will note that this machine does not have Roboto or Helvetica, so it used the fall back system sans-serif font [image: screenshot_20180907_105448] https://user-images.githubusercontent.com/2835826/45226403-ba908880-b28c-11e8-820f-ac1c3bd7d9ed.png

I did not modify any css rules, and the toolbar title looked correct on my end.

β€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ionic-team/ionic/issues/14707#issuecomment-419466370, or mute the thread https://github.com/notifications/unsubscribe-auth/ADmpTJM4D-1VO7Byu_paAE5v3NA5q5c4ks5uYom4gaJpZM4VCOK0 .

manucorporat commented 6 years ago

@mburger81 adding a font like Roboto is not free. We are working hard to keep app sizes low and adding a font unconditionally for all users regardless the font is already available in the system... or if the iOS mode is used is a bad solution.

However, @brandyscarney and I are working in a solution that makes every user happy

mburger81 commented 6 years ago

Yes you are righ on most parts.

But the bundle size is not so much important as the result of the look. It is important that the app is not downloading the font if not needed.

So how we have resolved this. We use the same roboto.scss as in v3 where we rename all the font family to RobotoX instead only Roboto

So when app is open nothing is downloaded. Now we check if app is material design and not running on android. If you are on android you know Rooboto is fine. If you are not on MD you know you don't need the font anyway.

For the rest we check if system has Roboto font (with check func) if it has it you know also in this case anything is fine.

If no Roboto font is present and you are not on Android and you are using MD for example on the webapp using Windows we overwrite the css variable for font-family with RobotoX. At this point browser ia downloading only required font for @font-face rule and anything is fine.

If you build android apk you can exclude roboto fonts if min sdk is 16+

This is a hack but it is working great without lack

Manu MA notifications@github.com schrieb am Fr., 7. Sep. 2018, 17:26:

@mburger81 https://github.com/mburger81 adding a font like Roboto is not free. We are working hard to keep app sizes low and adding a font unconditionally for all users regardless the font is already available in the system... or if the iOS mode is used is a bad solution.

However, @brandyscarney https://github.com/brandyscarney and I are working in a solution that makes every user happy

β€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ionic-team/ionic/issues/14707#issuecomment-419475209, or mute the thread https://github.com/notifications/unsubscribe-auth/ADmpTK008k8JjTcU1ej0TVfcsHmpNTM_ks5uYpA_gaJpZM4VCOK0 .

mburger81 commented 6 years ago

This is what google is saying about MD and system fonts

https://material.io/design/typography/understanding-typography.html#system-fonts Using system fonts Native system typefaces should be used for large blocks of text and any text below 14sp. 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.

So it seems, using system font on linux should be the right way using MD. But this sound really strange using MD weapps like calendar or gmail they download and use Roboto font instead of system found

In my personal opinion the app looks so incredible bad in linux comparing to v3 And it would be a big breaking change for the whole look

marticrespi commented 6 years ago

Same here, in v4 doesn't look as v3 with same configuration and same scss. Bigger icons, big font style..

v3 2018-09-08_09h59_35

v4 2018-09-08_10h02_03

And some others things for me are not working as expected. I can't change the color of icon tab, the old directive (col-3, col-6, etc) now with size="3" and others doesn't work as before..

mburger81 commented 6 years ago

@brandyscarney can we perhaps know if this is something we can expect to get work, or would you not support Roboto on NON Android machines out of the box, because no one has installed Roboto fonts on a PC (Windwos/Linux). In this case we have to go for a bad workaround. This means adding manually Roboto fonts on project and call it RobotoX and load them as default font if you are on a NON Android platform

thx

manucorporat commented 5 years ago

We will add an option to add roboto.css easily, but it will not be provided by default in order to keep the default installation as small and fast as possible! Going to work adding a new optional CSS for fonts asap!

brandyscarney commented 5 years ago

To track adding Roboto: https://github.com/ionic-team/ionic/issues/16938

ionitron-bot[bot] commented 5 years ago

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.