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

Turn off compatibility mode by default and turn on relative typography (rem) by default #981

Merged
merged 7 commits into from
Sep 6, 2018

Conversation

NickColley
Copy link
Contributor

@NickColley NickColley commented Sep 6, 2018

Closes #868

This pull request turns on relative typography by default unless the user has compatibility mode turned on.

It also turns off compatibility mode by default.

If a user wants to try relative typography in their application they can still do so.

We have just determined that we do not want to risk this by default since we can't be sure how the interface will behave with some elements resizing based on font size and some not.

I have tested this by confirming:

  1. When I add GOV.UK Template and Elements to the review application everything is very small
  2. When I add the compatibility variables it is as expected.
  3. Adding a test to check for 'rem;' in the output CSS.

https://trello.com/c/8iqDncHe/1389-2-enable-rem-based-typography-by-default

This feature will need to be enabled by users.
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-981 September 6, 2018 09:41 Inactive
@NickColley NickColley force-pushed the turn-on-responsive-typography-by-default branch from e4359d0 to a1970bb Compare September 6, 2018 09:45
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-981 September 6, 2018 09:45 Inactive
@NickColley NickColley force-pushed the turn-on-responsive-typography-by-default branch from a1970bb to cb477b4 Compare September 6, 2018 09:48
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-981 September 6, 2018 09:48 Inactive
@NickColley NickColley added this to the v2.0.0 milestone Sep 6, 2018
@NickColley NickColley changed the title [Do not merge] Turn off compatibility mode by default and turn on relative typography (rem) by default Turn off compatibility mode by default and turn on relative typography (rem) by default Sep 6, 2018
src/all.test.js Outdated
@@ -149,4 +149,21 @@ describe('GOV.UK Frontend', () => {

expect(functionCalls).toBeNull()
})
describe('responsive typography', async () => {
it('is enabled by default', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than adding these, can we update the tests in src/helpers/typography.test.js? (We currently explicitly disable responsive mode, and then have a test for when it's enabled, but we should probably flip that logic?)

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-981 September 6, 2018 10:25 Inactive
Ensures that new projects are using rem units for font sizes,
which allows end-users to adjust their font-size.

This is only enabled by default if the user is not using the compatibility mode variables.
Since it is now turned off by default, it needs documenting to explain
how to turn it on.
$govuk-typography-use-rem: false;
${sassBootstrap}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have moved these around to better demonstrate how our users would write their SCSS

@NickColley
Copy link
Contributor Author

@36degrees updated the tests with your feedback

Copy link
Contributor

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

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

Top stuff 👍

@NickColley NickColley merged commit 48c9751 into master Sep 6, 2018
@NickColley NickColley deleted the turn-on-responsive-typography-by-default branch September 6, 2018 10:31
@NickColley NickColley mentioned this pull request Sep 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants