feat(fonts): add lyon font #194

owensgit · 2021-02-03T10:53:37Z

Description

Adds the Lyon font with a suggested solution for not including it as part of the distributed Canopy package.

I've added an explanation to the USAGE.md doc and also to the Typography notes. Please take a look - hopefully the explanation is clear, but shout if not and I can improve it.

I also updated the Typography story view to display both fonts side by side with a pangram:

Without notes:

With notes:

Requirements

You can test that it works correctly by building the Canopy package locally, checking that the generated package does not include the Lyon font files. Then, copy the package the node_modules of your consuming application and follow the instructions to include the font in your assets directory. You should then be able to use the font modifier.

Storybook link: https://deploy-preview-194--legal-and-general-canopy.netlify.app/?path=/story/typography--headings

Checklist:

The commit messages follow the convention for this project
I have provided an adequate amount of test coverage
I have added the functionality to the test app
I have provided a story in storybook to document the changes
I have provided documentation in the notes section of the story
I have added any new public feature modules to public-api.ts
I have linked the new component to adobe DSM (if appropriate)

TODO

Add details of the Lyon font modifier to the notes
Add instructions for the license agreement
Use a regex for more precise targeting of the Lyon font in the Webpack build
Possibly add an Input() to the Heading component to apply the Lyon font (pending discussion)
A little more thorough testing - perhaps in other Angular projects
Add to the test app in a nicer way

elenagarrone · 2021-02-03T11:36:24Z

Could we use kebab case for the names of the fonts? just for consistency with roboto, e.g.:

roboto-latin-light.woff

elenagarrone · 2021-02-03T11:38:54Z

docs/USAGE.md

+
+When you include the canopy.css file in your project, it will try to load the font from that exact path. If your font files are in the wrong location, or have a typo in the filename, the Lyon font won't load correctly.
+
+#### How is Lyon omitted? 


I'm not sure we need this here? Users don't really have to know this 🤔

I was wondering about this too. In the end I decided to include it to help future developers, should they need to alter the Webpack config or add another commercial font. So if we were to keep it in some way, perhaps it should go into CONTRIBUTING.md - or maybe just remove it entirely 🤔 . What's your preference? I'm happy to go with the consensus on this (CC @thatguynamedandy, @paul-request and @aaronBery)

Maybe I'd move it to CONTRIBUTING.md

I think it would be better in CONTRIBUTING.md, or I would actually favour putting it in the git commit message (which is personal preference more than anything)

owensgit · 2021-02-03T13:53:33Z

Could we use kebab case for the names of the fonts? just for consistency with roboto, e.g.:
roboto-latin-light.woff

Done ✅

alexcanning · 2021-02-03T14:31:28Z

Presentation and layout looks good.

The example page is showing Lyon styles below .lg-font-size-4. However, the minimum size that should be used is .lg-font-size-4. I assume the method we're using to style in Lyon precludes us from removing the smaller sizes for Lyon, but removing the examples should be enough to stop people from using them. We'll also remove reference to them from DSM.

Here is are the specs for allowed Lyon styles:
https://legalandgeneral.invisionapp.com/d/main#/console/19910472/422088482/inspect

owensgit · 2021-02-03T14:45:34Z

Presentation and layout looks good.

The example page is showing Lyon styles below .lg-font-size-4. However, the minimum size that should be used is .lg-font-size-4. I assume the method we're using to style in Lyon precludes us from removing the smaller sizes for Lyon, but removing the examples should be enough to stop people from using them. We'll also remove reference to them from DSM.

Here is are the specs for allowed Lyon styles:
https://legalandgeneral.invisionapp.com/d/main#/console/19910472/422088482/inspect

Thanks Alex, those example have been removed 👍

andy-polhill · 2021-02-03T16:44:29Z

docs/USAGE.md

+
+Canopy consists of two fonts; Roboto and Lyon. Roboto is included in the distributed Canopy package, and will load automatically when you have installed Canopy correctly.
+
+Due to our commericial license agreement, Lyon is not included in the Canopy distributed package, so you need to include it in the **assets** directory of your application. Canopy still references Lyon in the \`\`@font-face\`\` rulesets, so you don't need to implement them, but you do need to ensure that the path to the font in your assets directory is the same as the paths specified by Canopy in \`\`@font-face\`\`.


Suggested change

Due to our commericial license agreement, Lyon is not included in the Canopy distributed package, so you need to include it in the **assets** directory of your application. Canopy still references Lyon in the \`\`@font-face\`\` rulesets, so you don't need to implement them, but you do need to ensure that the path to the font in your assets directory is the same as the paths specified by Canopy in \`\`@font-face\`\`.

Due to commercial licensing agreements the Lyon font is not included in the distributed package. You will need to manually add it to the **assets** directory of your application. Canopy does provide the css to work with the font and no additional code should be required. You will need to ensure the font is in the directory specified by Canopy.

Ah that's better, thanks Andy. Done ✅

andy-polhill · 2021-02-03T16:48:38Z

docs/USAGE.md

+
+Due to our commericial license agreement, Lyon is not included in the Canopy distributed package, so you need to include it in the **assets** directory of your application. Canopy still references Lyon in the \`\`@font-face\`\` rulesets, so you don't need to implement them, but you do need to ensure that the path to the font in your assets directory is the same as the paths specified by Canopy in \`\`@font-face\`\`.
+
+Canopy's path for Lyon is **/assets/fonts/lyon/**, therefore you need to put the font files in the following location:


I think there is a line to add here along the lines of...

Please ensure you have the correct licensing agreement in place before using the Lyon font in your application.

I would like to be able to link that text to somewhere, but not sure where.

andy-polhill · 2021-02-03T16:50:32Z

docs/USAGE.md

+
+When you include the canopy.css file in your project, it will try to load the font from that exact path. If your font files are in the wrong location, or have a typo in the filename, the Lyon font won't load correctly.
+
+#### How is Lyon omitted? 


I think it would be better in CONTRIBUTING.md, or I would actually favour putting it in the git commit message (which is personal preference more than anything)

andy-polhill · 2021-02-03T16:53:08Z

projects/canopy/src/styles/typography.notes.ts

+
+## Serving the Lyon font files
+
+Due to our commericial license agreement, Lyon is not included in the Canopy distributed package, so you need to include it in the **assets** directory of your application. Canopy still references Lyon in the \`\`@font-face\`\` rulesets, so you don't need to implement them, but you do need to ensure that the path to the font in your assets directory is the same as the paths specified by Canopy in \`\`@font-face\`\`.


The repetition of documentation between storybook and github is a bit annoying, but maybe one for a later date. Ideally all of our docs will be in one place (even if they are visible in Github and Storybook)

Yes good point. I wonder if we could have snippets of markdown that can be imported and used in more than one place if needed. Shall we capture this in an issue?

Could be worth tracking, a hard one to fix, but it might link partially to #35

andy-polhill · 2021-02-03T16:54:36Z

projects/canopy/src/styles/typography.scss

@@ -1,5 +1,53 @@
 @import '../styles/mixins.scss';
 $fonts-path: '../assets/fonts' !default;
+$fonts-path-lyon: '/assets/fonts' !default;


Because this is a default does that mean that people could potentially override this property if they are including the scss files? Just wondering if we document that?

I've removed the default, I think it just came from duplicating the line above to edit it. Having it there does indicate that this var could be changed, but in reality it should stay the same.

andy-polhill · 2021-02-03T16:56:00Z

webpack.global.config.js

+            loader: 'css-loader',
+            options: {
+              url: (url) => {
+                // TODO: use regex for more precise targeting


Have you done the TODO?

Yes, I did it but forgot to remove the comment 🤦

elenagarrone · 2021-02-03T17:58:52Z

docs/USAGE.md

+The custom Webpack config does two things when compiling Canopy's global CSS file (canopy.css):
+
+1. Prevents `css-loader` from transforming any URL referencing the Lyon font. This means the path will stay as relative to the base URL of the consuming application, therefore looking in the assets directory of consuming app.
+2. Removes the Lyon font files from the /dist directory after complication is complete, using `remove-files-webpack-plugin`. 


Typo: complication -> compilation

elenagarrone

Nice work @owensgit 🎊

package.json

projects/canopy/src/styles/variables/_typography.scss

webpack.global.config.js

The Roboto font is stored in **/projects/canopy/src/assets/fonts/roboto** as normal and is included in the distributed Canopy package. The Lyon font files are stored in **/assets/fonts** and ommitted from the distributed package via the Webpack config. The `css-loader` options prevent it from transforming any URL referencing the Lyon font. This means the path will stay as relative to the base URL of the consuming application (/assets/fonts), therefore looking in the assets directory of consuming app.

github-actions · 2021-02-05T16:24:31Z

🎉 This PR is included in version 5.1.0 🎉

The release is available on:

npm package (@latest dist-tag)
GitHub release

Your semantic-release bot 📦🚀

owensgit requested a review from a team as a code owner February 3, 2021 10:53

owensgit marked this pull request as draft February 3, 2021 10:53

owensgit force-pushed the lyon-font branch 2 times, most recently from f5fb072 to 3d3c528 Compare February 3, 2021 11:00

elenagarrone reviewed Feb 3, 2021

View reviewed changes

ABucin previously approved these changes Feb 3, 2021

View reviewed changes

owensgit dismissed ABucin’s stale review via dc9fdef February 3, 2021 13:55

owensgit force-pushed the lyon-font branch from 3d3c528 to dc9fdef Compare February 3, 2021 13:55

owensgit force-pushed the lyon-font branch 2 times, most recently from 592ea9d to e16609c Compare February 3, 2021 15:32

andy-polhill reviewed Feb 3, 2021

View reviewed changes

elenagarrone reviewed Feb 3, 2021

View reviewed changes

owensgit force-pushed the lyon-font branch from e16609c to a5724bd Compare February 4, 2021 10:40

owensgit marked this pull request as ready for review February 4, 2021 11:04

elenagarrone previously approved these changes Feb 4, 2021

View reviewed changes

owensgit dismissed elenagarrone’s stale review via e631230 February 4, 2021 12:49

owensgit force-pushed the lyon-font branch from a5724bd to e631230 Compare February 4, 2021 12:49

andy-polhill reviewed Feb 4, 2021

View reviewed changes

package.json Outdated Show resolved Hide resolved

andy-polhill reviewed Feb 4, 2021

View reviewed changes

projects/canopy/src/styles/variables/_typography.scss Show resolved Hide resolved

webpack.global.config.js Outdated Show resolved Hide resolved

owensgit force-pushed the lyon-font branch 3 times, most recently from ff87f80 to 6edb744 Compare February 5, 2021 09:54

owensgit force-pushed the lyon-font branch from 6edb744 to 67b18a2 Compare February 5, 2021 15:29

andy-polhill approved these changes Feb 5, 2021

View reviewed changes

paul-request approved these changes Feb 5, 2021

View reviewed changes

owensgit merged commit 1119e80 into Legal-and-General:master Feb 5, 2021

github-actions bot added the released label Feb 5, 2021

owensgit deleted the lyon-font branch May 12, 2021 14:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(fonts): add lyon font #194

feat(fonts): add lyon font #194

owensgit commented Feb 3, 2021 •

edited

Loading

elenagarrone commented Feb 3, 2021

elenagarrone Feb 3, 2021

owensgit Feb 3, 2021 •

edited

Loading

elenagarrone Feb 3, 2021

andy-polhill Feb 3, 2021

owensgit commented Feb 3, 2021

alexcanning commented Feb 3, 2021

owensgit commented Feb 3, 2021 •

edited

Loading

andy-polhill Feb 3, 2021

owensgit Feb 4, 2021

andy-polhill Feb 3, 2021

owensgit Feb 4, 2021

andy-polhill Feb 3, 2021

andy-polhill Feb 3, 2021

owensgit Feb 4, 2021

andy-polhill Feb 4, 2021

andy-polhill Feb 3, 2021

owensgit Feb 4, 2021

andy-polhill Feb 3, 2021

owensgit Feb 4, 2021

elenagarrone Feb 3, 2021

owensgit Feb 4, 2021

elenagarrone left a comment

github-actions bot commented Feb 5, 2021


		When you include the canopy.css file in your project, it will try to load the font from that exact path. If your font files are in the wrong location, or have a typo in the filename, the Lyon font won't load correctly.

		#### How is Lyon omitted?


		Canopy consists of two fonts; Roboto and Lyon. Roboto is included in the distributed Canopy package, and will load automatically when you have installed Canopy correctly.

		Due to our commericial license agreement, Lyon is not included in the Canopy distributed package, so you need to include it in the assets directory of your application. Canopy still references Lyon in the \`\`@font-face\`\` rulesets, so you don't need to implement them, but you do need to ensure that the path to the font in your assets directory is the same as the paths specified by Canopy in \`\`@font-face\`\`.


		Due to our commericial license agreement, Lyon is not included in the Canopy distributed package, so you need to include it in the assets directory of your application. Canopy still references Lyon in the \`\`@font-face\`\` rulesets, so you don't need to implement them, but you do need to ensure that the path to the font in your assets directory is the same as the paths specified by Canopy in \`\`@font-face\`\`.

		Canopy's path for Lyon is /assets/fonts/lyon/, therefore you need to put the font files in the following location:


		## Serving the Lyon font files

		Due to our commericial license agreement, Lyon is not included in the Canopy distributed package, so you need to include it in the assets directory of your application. Canopy still references Lyon in the \`\`@font-face\`\` rulesets, so you don't need to implement them, but you do need to ensure that the path to the font in your assets directory is the same as the paths specified by Canopy in \`\`@font-face\`\`.

feat(fonts): add lyon font #194

feat(fonts): add lyon font #194

Conversation

owensgit commented Feb 3, 2021 • edited Loading

Description

Requirements

Checklist:

TODO

elenagarrone commented Feb 3, 2021

Choose a reason for hiding this comment

owensgit Feb 3, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

owensgit commented Feb 3, 2021

alexcanning commented Feb 3, 2021

owensgit commented Feb 3, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elenagarrone left a comment

Choose a reason for hiding this comment

github-actions bot commented Feb 5, 2021

owensgit commented Feb 3, 2021 •

edited

Loading

owensgit Feb 3, 2021 •

edited

Loading

owensgit commented Feb 3, 2021 •

edited

Loading