Skip to content
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

Allow the toolkit font to be changed #230

Merged
merged 1 commit into from Apr 18, 2016
Merged

Allow the toolkit font to be changed #230

merged 1 commit into from Apr 18, 2016

Conversation

@fofr
Copy link
Contributor

@fofr fofr commented Nov 19, 2015

Some projects, such as petitions and admin tools aren’t GOVUK branded but still use the toolkit. These projects sometimes can’t use New Transport, or choose not to.

  • Make it easy to switch out the font used
  • Add documentation
  • Write font stacks on single line as that’s how they are usually
    presented in CSS, etc.

cc @quis @robinwhittleton

@dsingleton
Copy link
Contributor

@dsingleton dsingleton commented Nov 19, 2015

💯 👍 to the idea. This will save quite a few projects having to outright fork the toolkit to make this change.

$Helvetica-Regular:
"GDS-Logo",
sans-serif;
$Helvetica-Regular: "GDS-Logo", sans-serif;

This comment has been minimized.

@gemmaleigh

gemmaleigh Nov 19, 2015
Contributor

It might help to move this above, to sit with the other Sass variables for font stacks.

This comment has been minimized.

@fofr

fofr Nov 20, 2015
Author Contributor

I think it needs to stay below the @font-face declaration, but I guess both blocks could move up? (There's no reference to this within the toolkit, but it's used in a couple of apps for organisation logo styles).

This comment has been minimized.

@gemmaleigh

gemmaleigh Nov 20, 2015
Contributor

Should all the @font-face rules sit together? The rest are in the govuk template:
https://github.com/alphagov/govuk_template/blob/master/source/assets/stylesheets/fonts.css

This comment has been minimized.

@alextea

alextea Nov 20, 2015
Contributor

@gemmaleigh I think the fonts aren't in here deliberately, to avoid licensing issues. I've just had a discussion about this on slack: https://ukgovernmentdigital.slack.com/archives/frontend/p1448023249000062

@fofr fofr changed the title [DISCUSS] Allow the toolkit font to be changed Allow the toolkit font to be changed Nov 20, 2015
@fofr
Copy link
Contributor Author

@fofr fofr commented Nov 24, 2015

Closing until I have addressed @robinwhittleton's feedback.

@fofr fofr closed this Nov 24, 2015
@fofr fofr reopened this Apr 18, 2016
Some projects, such as petitions and admin tools aren’t GOVUK branded
but still use the toolkit. These projects sometimes can’t use New
Transport, or sometimes choose not to.

* Make it easy to switch out the font used
* Add documentation
* Write font stacks on single line as that’s how they are usually
presented in CSS.
@fofr fofr force-pushed the change-font-stack branch from e3dbaad to 3bf7b9e Apr 18, 2016
@fofr
Copy link
Contributor Author

@fofr fofr commented Apr 18, 2016

I've rebased to remove conflicts with master and updated documentation to include @robinwhittleton’s changes.

@dsingleton
Copy link
Contributor

@dsingleton dsingleton commented Apr 18, 2016

👍

Longer term i've been thinking we make make all our variables overridable by default - possibly pulling them out in to a separate module for visibility (or using SASS doc to expose them)

@fofr fofr merged commit f8530a0 into master Apr 18, 2016
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@fofr fofr deleted the change-font-stack branch Apr 18, 2016
fofr added a commit that referenced this pull request Apr 18, 2016
- Allow New Transport font stack to be overridden by apps (PR #230)
@fofr fofr mentioned this pull request Apr 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.