Skip to content
This repository has been archived by the owner on Jan 25, 2021. It is now read-only.

Change "Google Fonts" to "Font Schemes", multiple enhancements and clean up #61

Merged

Conversation

richard67
Copy link
Member

@richard67 richard67 commented Sep 6, 2020

Pull Request for Issue #32 .

Replaces PR #50 .

Summary of Changes

  • Remove previosly added local fonts.
  • Do not use build/media_source/fonts and media/fonts anymore.
  • Only use local fonts which are already available in the core - currently this is only Roboto, which is used by Atum and comes as NPM dependency).
  • Add back the possibility to use fonts from web.
  • Add 2 font schemes using Google Fonts from web for testing the new design.
  • Use CSS variables to set font families and weights for headings and body - more variables might be added later with this PR or future PR while the final design is being implemented.
  • Simplify UX: Only one grouped list field for selecting the font schemes or no font scheme, ie no need to first switch it on with one switcher and then in another field selecting the font scheme, it can be done in one step now.
  • Remove the description from the remaining field.
  • Show a warning note about possible GDPR issues with fonts from web and possible performance issues with fonts from local folders.

Note: The font schemes and the design of the template is still subject of change, so this PR does not show the final look regarding fonts.

If possible we should make a PR to add "Noto Sans" as a 2nd local font with another NPM dependency due to its great support for non-latin character sets.

This would also allow us to have the same look with local fonts and fonts from web when using "Rototo" and "Noto Sans" with the new design.

Testing Instructions

Test 1: Without this Pull Request

  1. Checkout the development branch, pull the latest changes and run "npm ci".
    Or use the following package to make a new installation:
    https://test5.richard-fath.de/Joomla_4.0.0-beta4-dev-Cassiopeia-Test-Full_Package_2020-09-07_v1.zip
    Or use the following package to update a current 4.0-dev or latest 4.0 nightly build from 2020-09-07 or a bit earlier, but not too old:
    https://test5.richard-fath.de/Joomla_4.0.0-beta4-dev-Cassiopeia-Test-Update_Package_2020-09-07_v1.zip

  2. Login to backend.

  3. Edit the Cassiopeia template style and check the "Advanced" Tab.

Result: See section "Actual result BEFORE applying this Pull Request" below.

  1. Check on the server if there is a sub-folder "fonts" in the "media" folder.

Result: There is such a folder which contains css, woff and woff2 files.

Test 2: With this Pull Request

  1. On the installation used in the previous test, apply the changes of this PR and run npm ci.
    Alternatively, if you don't know how to do that, use following package to make a new installation:
    https://test5.richard-fath.de/Joomla_4.0.0-beta4-dev-Cassiopeia-Test-Full_Package_2020-09-07_v2_PR-61.zip

  2. Login to backend.

  3. Edit the Cassiopeia template style and check the "Advanced" Tab.

Result: See section "Expected result AFTER applying this Pull Request" below.

  1. Check on the server if there is a sub-folder "fonts" in the "media" folder.

Result: There is no such a folder.

  1. Try out the different font schemes.
    Change the font scheme in backend, then reload the page in frontend to see if fonts change.
    Inspect the frontend page's header in your browser tool that the css for the font scheme is loaded, if not "None" has been selected in backend.

  2. Compare the branch of this PR with the 4.0-dev branch of this repository by following this link: 4.0-dev...richard67:development-fonts-enhancements.
    This shows all changes in the current development branch plus the changes of this PR applied, compared with 4.0-dev.
    Verify that this comoparison doesn't show any new fonts or external dependencies being added to 4.0-dev.

Actual result BEFORE applying this Pull Request

  • Edit Cassiopeia's Template Style:
    cassiopeia-pr-61_1

  • Font files and css which have to be maintaned manually will be added to 4.0-dev with our current development.

Expected result AFTER applying this Pull Request

  • Edit Cassiopeia's Template Style:
    cassiopeia-pr-61_2

Note: The screenshot above doesn't show the latest correction for capitalisation of the "Fonts Scheme" label.

  • Font Schemes:
    cassiopeia-pr-61_3

  • No font files and css which have to be maintaned manually and no new external dependencies will be added to 4.0-dev with our current development when this PR has been merged into the development branch.

Documentation Changes Required

None beside what has to be done for the new option anyway.

@richard67 richard67 changed the title [WiP] Change "Google Fonts" to "Font Schemes", multiple enhancements and clean up Change "Google Fonts" to "Font Schemes", multiple enhancements and clean up Sep 6, 2020
@richard67 richard67 marked this pull request as ready for review September 6, 2020 01:20
@richard67
Copy link
Member Author

@SharkyKZ Will this PR fix the issues you had identified for our development here for the local fonts?

@chmst
Copy link
Collaborator

chmst commented Sep 6, 2020

Tested all variants with sample data. No Problems, headers and text fonts appear as expected. It is an important part of the documentation of cassiopeia..

@richard67
Copy link
Member Author

@chmst Thanks for test and feedback. If you reviewed the code, too, you can use the review function on GitHub to approve it. That would be cool.

@chmst chmst mentioned this pull request Sep 6, 2020
@richard67
Copy link
Member Author

Thanks @chmst . Now one more tester would be nice.

Copy link
Collaborator

@hans2103 hans2103 left a comment

Choose a reason for hiding this comment

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

CSS Variables will not work in IE11
https://caniuse.com/?search=css%20variables

But... we don't have to write code for IE11 anymore in J4
https://developer.joomla.org/news/788-joomla-4-on-the-move.html

Drop support for Internet Explorer

The decision to drop support for IE was based on a few criteria. Less than 5% of the internet users browse the web with Internet Explorer. The new Edge browser (based on Chromium) will have an IE compatibility layer that will drive IE usage further down.

Does this mean that Joomla 4 will not work in IE? Yes and No!

The Joomla 4 backend template won’t support IE, therefore you’ll need another browser to install/maintain a website. The same is true for the core frontend template, Cassiopeia, as it will not support the IE requirements, so you can expect some display issues. However, it can be solved by creating or installing a template that supports IE.

@richard67
Copy link
Member Author

@brianteeman Could you review the language strings for me?

https://github.com/joomla/cassiopeia/pull/61/files#diff-768a2ef4d1becd1fb6bcc68ff592c9b7R15-R18

And is display of the yellow warning ok, or should it be changed to something else? It should be something which we can define in the form XML, like the note field used here, and not have to be added add with PHP. Or is a note field ok but I should change it from warning to notice?

Thanks in advance.

@brianteeman
Copy link
Contributor

I can help with the wording but I want to check

  1. dont you mean privacy issue not data protection
  2. why is it a performance issue on high traffic sites

@richard67
Copy link
Member Author

@brianteeman

1. dont you mean privacy issue not data protection

I mean a general word for what GDPR is in the EU.

We had discussed in the team if it makes sens to show a warning like that regarding possible GDPR violations when using Google Fonts.

I just wanted to describe it more general so it also fits to other countries like EU, if those establish such rules.

2. why is it a performance issue on high traffic sites

Wasn't it you who commented here in one of the PRs about local fonts that it is a performance issue if many visitors load fonts from the site's web server?

That's why I have added that sentence to the warning.

Let me knof it it shall be removed or changed.

Thanks in advance.

@brianteeman
Copy link
Contributor

I would prefer the term "privacy" than "data protection" - it works better globally

The performance issue I referred to is for ALL sites because the user is downloading the font from a server that may not be local (ie not a cdn) or that isnt very fast. So it impacts all sites irrespective of their popularity

@richard67
Copy link
Member Author

@brianteeman What is better, "privacy rules" or "privacy laws"?

@brianteeman
Copy link
Contributor

brianteeman commented Sep 7, 2020

1st suggestion
TPL_CASSIOPEIA_FONT_NOTE_TEXT="Loading fonts from external sources might be against privacy regulations in some countries.<br>Loading fonts from a local folder might have a performance impact on your site."

@richard67
Copy link
Member Author

@brianteeman Thanks a lot. Have corrected as suggested. Any further suggestions?

@richard67 richard67 merged commit fde0291 into joomla:development Sep 7, 2020
@richard67 richard67 deleted the development-fonts-enhancements branch September 7, 2020 15:40
@richard67
Copy link
Member Author

Merging for now. For further issues and enhancements please open a new issue, or alternatively comment in our dummy PR #48 for testing all the development.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants