Opt-in global anchor and paragraph styles #658

kr8n3r · 2018-04-17T10:06:25Z

To ease prototyping and new production instances we provided a way to include global styles for links and paragraphs, so that users do not have to add classes to every paragraph or link.

Inclusion is controlled by $govuk-global-styles variable in globals/settings folder.
Documentation has been added to the main README.md file

If Elements or Template compatibility variables are set to true then we render fixes to those and also include global styles

Trello: https://trello.com/c/toKpf2vV/868-3-add-optional-styles-that-set-paragraph-and-link-styles

alex-ju

Left two comment. The second one is non-blocking and more of a personal opinion. Otherwise it's good to go. I'll wait for other to have a look, since it's more about agreeing on the approach.

alex-ju · 2018-04-19T12:47:55Z

docs/global-styles.md

+
+In GOV.UK Frontend we want to provide a convenient way to have correct styles applied to your links (`<a>`) and paragraphs (`<p>`) without having to set required classes for each instance of an element.
+
+We have chosen 'Opt-in' as the default behaviour.


Shall we say 'Opted-in'. For me 'Opt-in' sound like I have the option to choose if I want it, which is the opposite of what we're doing by default. We can ask @amyhupe to help with wording.

alex-ju · 2018-04-19T12:53:54Z

src/all/_all.scss

 // Settings
 @import "../globals/common";
+@import "../globals/optional/global-styles";


Given the context optional/ may be the right name.
My view of this was that we'd have a /core in which we place the default style that you can choose not to use.

was considering putting it in core first. i have no issue with it being in core tbh

I quite like optional/ as this clearly differentiates those styles from the rest. But not a blocker.

hannalaakso · 2018-04-19T15:35:59Z

docs/global-styles.md

+@import "govuk-frontend/all/all";
+```
+If you choose this option, you will need to either use classes on links (`<a>`) and paragraphs (`<p>`) or have a different way to apply correct styles.
+


It would be good to document here, or link from here to another doc, what to look out for if choosing to set $govuk-global-styles to false and adding our link classes selectively, eg. govuk-link class must be added along with govuk-link--muted.

Instead of "use classes", maybe "add GOV.UK Frontend classes"?

NickColley · 2018-04-19T15:38:23Z

docs/global-styles.md

@@ -0,0 +1,20 @@
+# Global Styles


I think this documentation is most likely to be useful if you're using the legacy stuff, with this in mind should we align this work with the compatibility mode stuff?

User scenario:

I have GOV.UK Template, and just want a build that works with that

in that case you don't need global styles then, right? As they're coming from template?

all of the *compatibillity vars are set to true in Frontend 🤔

NickColley · 2018-04-25T16:28:50Z

src/globals/core/_global-styles.scss

+  //
+  // 4. If both $govuk-global-styles and $govuk-compatibility-govuktemplate are set to false
+  // then don't include global styles
+  @if global-variable-exists("govuk-global-styles") == false or $govuk-global-styles == true or $govuk-compatibility-govuktemplate == true {


should this be $govuk-compatibility-govuktemplate == false?

(and would it need the global-variable-exists statement too?)

NickColley

Here's my understanding:

If we agree that global styles should be enabled for new projects (or prototyping) this should be the default.

Then if people need to use GOV.UK Frontend in a way that's compatible with the older projects they should opt in.

With this in mind.

All compatibility flags should be false by default.

global styles should be true by default

govuktemplate flag should set global styles to false

global styles should still be able to be set on it's own, if the user is using GOV.UK Frontend in a project that uses another framework for example.

so at the risk of complicating this PR, should we need to update the defaults for the compatibility flags?

joelanman · 2018-04-25T17:05:35Z

It would be good to go over this - I'm finding it hard to understand the logic and naming - especially $govuk-compatibility-govuktemplate

NickColley · 2018-05-02T12:18:59Z

src/all/all.test.js

+      const results = await sassRender({ data: sass, ...sassConfig })
+      expect(results.css.toString()).toContain(', p {')
+    })
+    it('for links and paragraphs are not output if flag is set to false', async () => {


Could we also have variants for

When compatibility flags are set to true

When compatibility flags are set to true and global flag is set to true

joelanman · 2018-05-02T13:10:54Z

it would be great to get a diagram to explain the logic for this work - happy to help out if needed

kr8n3r · 2018-05-03T07:35:36Z

@dashouse made a point that if you are using template then you might need our globals as we have changed spacing in typography

kr8n3r · 2018-05-03T09:52:44Z

updated to just include optional global styles based on a flag

36degrees

💯 👍 👏

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-658 April 17, 2018 10:06 Inactive

kr8n3r force-pushed the global-a-and-p-styles branch from 789ab3a to 8ad882d Compare April 17, 2018 10:23

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-658 April 17, 2018 10:23 Inactive

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-658 April 19, 2018 10:51 Inactive

alex-ju reviewed Apr 19, 2018

View reviewed changes

kr8n3r force-pushed the global-a-and-p-styles branch from 4f1d1c3 to 910f4d7 Compare April 19, 2018 14:09

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-658 April 19, 2018 14:09 Inactive

kr8n3r force-pushed the global-a-and-p-styles branch from 910f4d7 to 32a588f Compare April 19, 2018 14:56

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-658 April 19, 2018 14:56 Inactive

hannalaakso reviewed Apr 19, 2018

View reviewed changes

NickColley reviewed Apr 19, 2018

View reviewed changes

kr8n3r force-pushed the global-a-and-p-styles branch from 32a588f to de4bdf9 Compare April 23, 2018 07:49

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-658 April 23, 2018 07:49 Inactive

kr8n3r force-pushed the global-a-and-p-styles branch from de4bdf9 to 5920d0d Compare April 25, 2018 15:19

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-658 April 25, 2018 15:19 Inactive

kr8n3r force-pushed the global-a-and-p-styles branch from 5920d0d to 8818d78 Compare April 25, 2018 15:20

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-658 April 25, 2018 15:20 Inactive

kr8n3r force-pushed the global-a-and-p-styles branch from 8818d78 to 39defd2 Compare April 25, 2018 15:24

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-658 April 25, 2018 15:24 Inactive

kr8n3r force-pushed the global-a-and-p-styles branch from 39defd2 to 9a099d1 Compare April 25, 2018 15:33

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-658 April 25, 2018 15:33 Inactive

kr8n3r force-pushed the global-a-and-p-styles branch from 9a099d1 to f5955d6 Compare April 25, 2018 15:45

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-658 April 25, 2018 15:45 Inactive

kr8n3r force-pushed the global-a-and-p-styles branch from f5955d6 to c000305 Compare April 25, 2018 15:57

kr8n3r changed the title ~~WIP: Global a and p styles~~ Global a and p styles Apr 25, 2018

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-658 April 25, 2018 15:57 Inactive

NickColley reviewed Apr 25, 2018

View reviewed changes

kr8n3r force-pushed the global-a-and-p-styles branch from c000305 to 9837953 Compare April 25, 2018 20:11

kr8n3r force-pushed the global-a-and-p-styles branch 3 times, most recently from 9adb06c to 27a5744 Compare May 2, 2018 11:32

NickColley reviewed May 2, 2018

View reviewed changes

kr8n3r force-pushed the global-a-and-p-styles branch 3 times, most recently from dd5bfd3 to c13c6ec Compare May 2, 2018 16:11

kr8n3r force-pushed the global-a-and-p-styles branch 7 times, most recently from 08bf080 to e837894 Compare May 3, 2018 09:50

kr8n3r changed the title ~~Global a and p styles and legacy compatibility setting~~ Opt-in global a and p styles May 3, 2018

kr8n3r changed the title ~~Opt-in global a and p styles~~ Opt-in global link and anchor styles May 3, 2018

Jani Kraner added 3 commits May 3, 2018 11:05

Global styles inclusion based on flag

27f29ce

Document in CHANGELOG

f4d258a

Document settings in README

e32fd39

kr8n3r force-pushed the global-a-and-p-styles branch from e837894 to cf119ae Compare May 3, 2018 10:05

Add tests to check output

563d373

kr8n3r force-pushed the global-a-and-p-styles branch from cf119ae to 563d373 Compare May 3, 2018 11:00

36degrees approved these changes May 3, 2018

View reviewed changes

kr8n3r changed the title ~~Opt-in global link and anchor styles~~ Opt-in global anchor and paragraph styles May 3, 2018

kr8n3r merged commit d1595d6 into master May 3, 2018

36degrees deleted the global-a-and-p-styles branch May 3, 2018 12:58

kr8n3r mentioned this pull request May 18, 2018

Release v0.0.29-alpha #700

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Opt-in global anchor and paragraph styles #658

Opt-in global anchor and paragraph styles #658

kr8n3r commented Apr 17, 2018 •

edited

Loading

alex-ju left a comment

alex-ju Apr 19, 2018

alex-ju Apr 19, 2018

kr8n3r Apr 19, 2018

hannalaakso Apr 19, 2018

hannalaakso Apr 19, 2018

NickColley Apr 19, 2018

kr8n3r Apr 20, 2018

kr8n3r Apr 23, 2018

NickColley Apr 25, 2018 •

edited

Loading

NickColley left a comment

joelanman commented Apr 25, 2018

NickColley May 2, 2018

joelanman commented May 2, 2018

kr8n3r commented May 3, 2018

kr8n3r commented May 3, 2018

36degrees left a comment


		In GOV.UK Frontend we want to provide a convenient way to have correct styles applied to your links (`<a>`) and paragraphs (`<p>`) without having to set required classes for each instance of an element.

		We have chosen 'Opt-in' as the default behaviour.

Opt-in global anchor and paragraph styles #658

Opt-in global anchor and paragraph styles #658

Conversation

kr8n3r commented Apr 17, 2018 • edited Loading

alex-ju left a 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

NickColley Apr 25, 2018 • edited Loading

Choose a reason for hiding this comment

NickColley left a comment

Choose a reason for hiding this comment

joelanman commented Apr 25, 2018

Choose a reason for hiding this comment

joelanman commented May 2, 2018

kr8n3r commented May 3, 2018

kr8n3r commented May 3, 2018

36degrees left a comment

Choose a reason for hiding this comment

kr8n3r commented Apr 17, 2018 •

edited

Loading

NickColley Apr 25, 2018 •

edited

Loading