Remove prose scope #778
Merged
Remove prose scope #778
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-review-pr-778
36degrees
reviewed
Jun 8, 2018
CHANGELOG.md
Outdated
| @@ -113,6 +113,16 @@ Note: We're not following semantic versioning yet, we are going to talk about th | |||
| Note that this function will return 'web-safe' colours by default. You can | |||
| pass $websafe: false to get the non-websafe colour. | |||
|
|
|||
| - Removal of `govuk-prose-scope` | |||
| We don't have confidence that the prose scope is well understood – without further research / better documentation it's safer to remove it for now. | |||
There was a problem hiding this comment.
Total nitpick - but can we please wrap at :80 consistently? The next paragraph is wrapped, but this paragraph and the final paragraph are not.
There was a problem hiding this comment.
(Good changelog entry BTW!)
kr8n3r
force-pushed
the
remove-prose-scope
branch
from
4de4a6d
to
709d2ab
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-review-pr-778
kr8n3r
force-pushed
the
remove-prose-scope
branch
from
709d2ab
to
00517e6
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-review-pr-778
|
Looks like there's still a few references to the prose scope in |
kr8n3r
force-pushed
the
remove-prose-scope
branch
from
00517e6
to
e01781c
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-review-pr-778
added 2 commits
June 13, 2018 10:00
kr8n3r
force-pushed
the
remove-prose-scope
branch
from
e01781c
to
177c7ac
Compare
|
good spot. updated |
36degrees
approved these changes
Jun 13, 2018
Merged
|
FWIW, I thought this was a really neat feature. There are similar, generic classes in other css frameworks. I thought these were fairly well understood. e.g. bulma. A single class to handle WYSIWYG generated content, where only HTML tags are available is a common use case . +1 for it's return! |
|
Thanks for the feedback @davidjamesstone – that's really useful to know. We've not ruled out adding prose scope or at least something very similar back in in the future. It's also good to see examples of how this works in other systems, like in the Bulma documentation you linked to. |
kevindew
added a commit
to alphagov/govuk_publishing_components
that referenced
this pull request
Apr 30, 2019
This removes the dependency this gem has on govspeak. This is in preparation of for govspeak to have the ability to call this gem as a means to render components within govspeak. This avoids a cyclic dependency. This gem previously used govspeak as a means to render markdown, there wasn't actually any specific reason this was govspeak - probably convenience more than anything. So I've stripped this back to use the Kramdown gem directly. I did debate whether to use commonmarker [1] instead of kramdown, but felt that since there's a high chance govspeak will be installed in places of this gem they could share the same markdown library. For styling I created a SASS mixin that could be used for markdown typography and included in govspeak. This allows the styles to be shared. I feel in time these may diverge slightly to allow features govspeak disables like more headers, but this didn't seem necessary to implement now. For the wrapper for markdown HTML I went for a class of `component-markdown` on the basis that it is intended to have the results of markdown HTML inside it. I did consider using the term "prose" instead of "markdown", as that is govuk-frontend terminology for HTML without classes. However govuk-frontend have since removed this [2] so I felt using the name "markdown" would be easier to understand. As I put this together I actually think there's value to be unlocked from having means to render markdown in apps and govspeak be two separate things. We have to disable govspeak security features to allow generic app rendering [3] so a divergence of these would likely allow both govspeak security and in app HTML flexibility to increase. Without the baggage of govspeak options we could also just allow passing raw markdown direct to a component rather than having an app convert it as is done with govspeak. I've not taken these thoughts any further here though, I'll see how this pans out and if these are real needs. [1]: https://github.com/gjtorikian/commonmarker [2]: alphagov/govuk-frontend#778 [3]: https://github.com/alphagov/content-publisher/blob/654bf1a257e9a0de607e2c16b61754d55f5f8a72/app/helpers/application_helper.rb#L9
kevindew
added a commit
to alphagov/govuk_publishing_components
that referenced
this pull request
Apr 30, 2019
This removes the dependency this gem has on govspeak. This is in preparation of for govspeak to have the ability to call this gem as a means to render components within govspeak. This avoids a cyclic dependency. This gem previously used govspeak as a means to render markdown, there wasn't actually any specific reason this was govspeak - probably convenience more than anything. So I've stripped this back to use the Kramdown gem directly. I did debate whether to use commonmarker [1] instead of kramdown, but felt that since there's a high chance govspeak will be installed in places of this gem they could share the same markdown library. For styling I created a SASS mixin that could be used for markdown typography and included in govspeak. This allows the styles to be shared. I feel in time these may diverge slightly to allow features govspeak disables like more headers, but this didn't seem necessary to implement now. For the wrapper for markdown HTML I went for a class of `component-markdown` on the basis that it is intended to have the results of markdown HTML inside it. I did consider using the term "prose" instead of "markdown", as that is govuk-frontend terminology for HTML without classes. However govuk-frontend have since removed this [2] so I felt using the name "markdown" would be easier to understand. As I put this together I actually think there's value to be unlocked from having means to render markdown in apps and govspeak be two separate things. We have to disable govspeak security features to allow generic app rendering [3] so a divergence of these would likely allow both govspeak security and in app HTML flexibility to increase. Without the baggage of govspeak options we could also just allow passing raw markdown direct to a component rather than having an app convert it as is done with govspeak. I've not taken these thoughts any further here though, I'll see how this pans out and if these are real needs. [1]: https://github.com/gjtorikian/commonmarker [2]: alphagov/govuk-frontend#778 [3]: https://github.com/alphagov/content-publisher/blob/654bf1a257e9a0de607e2c16b61754d55f5f8a72/app/helpers/application_helper.rb#L9
kevindew
added a commit
to alphagov/govuk_publishing_components
that referenced
this pull request
Apr 30, 2019
This removes the dependency this gem has on govspeak. This is in preparation of for govspeak to have the ability to call this gem as a means to render components within govspeak. This avoids a cyclic dependency. This gem previously used govspeak as a means to render markdown, there wasn't actually any specific reason this was govspeak - probably convenience more than anything. So I've stripped this back to use the Kramdown gem directly. I did debate whether to use commonmarker [1] instead of kramdown, but felt that since there's a high chance govspeak will be installed in places of this gem they could share the same markdown library. For styling I created a SASS mixin that could be used for markdown typography and included in govspeak. This allows the styles to be shared. I feel in time these may diverge slightly to allow features govspeak disables like more headers, but this didn't seem necessary to implement now. For the wrapper for markdown HTML I went for a class of `component-markdown` on the basis that it is intended to have the results of markdown HTML inside it. I did consider using the term "prose" instead of "markdown", as that is govuk-frontend terminology for HTML without classes. However govuk-frontend have since removed this [2] so I felt using the name "markdown" would be easier to understand. As I put this together I actually think there's value to be unlocked from having means to render markdown in apps and govspeak be two separate things. We have to disable govspeak security features to allow generic app rendering [3] so a divergence of these would likely allow both govspeak security and in app HTML flexibility to increase. Without the baggage of govspeak options we could also just allow passing raw markdown direct to a component rather than having an app convert it as is done with govspeak. I've not taken these thoughts any further here though, I'll see how this pans out and if these are real needs. [1]: https://github.com/gjtorikian/commonmarker [2]: alphagov/govuk-frontend#778 [3]: https://github.com/alphagov/content-publisher/blob/654bf1a257e9a0de607e2c16b61754d55f5f8a72/app/helpers/application_helper.rb#L9
kevindew
added a commit
to alphagov/govuk_publishing_components
that referenced
this pull request
Apr 30, 2019
This removes the dependency this gem has on govspeak. This is in preparation of for govspeak to have the ability to call this gem as a means to render components within govspeak. This avoids a cyclic dependency. This gem previously used govspeak as a means to render markdown, there wasn't actually any specific reason this was govspeak - probably convenience more than anything. So I've stripped this back to use the Kramdown gem directly. I did debate whether to use commonmarker [1] instead of kramdown, but felt that since there's a high chance govspeak will be installed in places of this gem they could share the same markdown library. For styling I created a SASS mixin that could be used for markdown typography and included in govspeak. This allows the styles to be shared. I feel in time these may diverge slightly to allow features govspeak disables like more headers, but this didn't seem necessary to implement now. For the wrapper for markdown HTML I went for a class of `component-markdown` on the basis that it is intended to have the results of markdown HTML inside it. I did consider using the term "prose" instead of "markdown", as that is govuk-frontend terminology for HTML without classes. However govuk-frontend have since removed this [2] so I felt using the name "markdown" would be easier to understand. As I put this together I actually think there's value to be unlocked from having means to render markdown in apps and govspeak be two separate things. We have to disable govspeak security features to allow generic app rendering [3] so a divergence of these would likely allow both govspeak security and in app HTML flexibility to increase. Without the baggage of govspeak options we could also just allow passing raw markdown direct to a component rather than having an app convert it as is done with govspeak. I've not taken these thoughts any further here though, I'll see how this pans out and if these are real needs. [1]: https://github.com/gjtorikian/commonmarker [2]: alphagov/govuk-frontend#778 [3]: https://github.com/alphagov/content-publisher/blob/654bf1a257e9a0de607e2c16b61754d55f5f8a72/app/helpers/application_helper.rb#L9
kevindew
added a commit
to alphagov/govuk_publishing_components
that referenced
this pull request
May 1, 2019
This removes the dependency this gem has on govspeak. This is in preparation of for govspeak to have the ability to call this gem as a means to render components within govspeak. This avoids a cyclic dependency. This gem previously used govspeak as a means to render markdown, there wasn't actually any specific reason this was govspeak - probably convenience more than anything. So I've stripped this back to use the Kramdown gem directly. I did debate whether to use commonmarker [1] instead of kramdown, but felt that since there's a high chance govspeak will be installed in places of this gem they could share the same markdown library. For styling I created a SASS mixin that could be used for markdown typography and included in govspeak. This allows the styles to be shared. I feel in time these may diverge slightly to allow features govspeak disables like more headers, but this didn't seem necessary to implement now. For the wrapper for markdown HTML I went for a class of `component-markdown` on the basis that it is intended to have the results of markdown HTML inside it. I did consider using the term "prose" instead of "markdown", as that is govuk-frontend terminology for HTML without classes. However govuk-frontend have since removed this [2] so I felt using the name "markdown" would be easier to understand. As I put this together I actually think there's value to be unlocked from having means to render markdown in apps and govspeak be two separate things. We have to disable govspeak security features to allow generic app rendering [3] so a divergence of these would likely allow both govspeak security and in app HTML flexibility to increase. Without the baggage of govspeak options we could also just allow passing raw markdown direct to a component rather than having an app convert it as is done with govspeak. I've not taken these thoughts any further here though, I'll see how this pans out and if these are real needs. [1]: https://github.com/gjtorikian/commonmarker [2]: alphagov/govuk-frontend#778 [3]: https://github.com/alphagov/content-publisher/blob/654bf1a257e9a0de607e2c16b61754d55f5f8a72/app/helpers/application_helper.rb#L9
kevindew
added a commit
to alphagov/govuk_publishing_components
that referenced
this pull request
May 1, 2019
This removes the dependency this gem has on govspeak. This is in preparation of for govspeak to have the ability to call this gem as a means to render components within govspeak. This avoids a cyclic dependency. This gem previously used govspeak as a means to render markdown, there wasn't actually any specific reason this was govspeak - probably convenience more than anything. So I've stripped this back to use the Kramdown gem directly. I did debate whether to use commonmarker [1] instead of kramdown, but felt that since there's a high chance govspeak will be installed in places of this gem they could share the same markdown library. For styling I created a SASS mixin that could be used for markdown typography and included in govspeak. This allows the styles to be shared. I feel in time these may diverge slightly to allow features govspeak disables like more headers, but this didn't seem necessary to implement now. For the wrapper for markdown HTML I went for a class of `component-markdown` on the basis that it is intended to have the results of markdown HTML inside it. I did consider using the term "prose" instead of "markdown", as that is govuk-frontend terminology for HTML without classes. However govuk-frontend have since removed this [2] so I felt using the name "markdown" would be easier to understand. As I put this together I actually think there's value to be unlocked from having means to render markdown in apps and govspeak be two separate things. We have to disable govspeak security features to allow generic app rendering [3] so a divergence of these would likely allow both govspeak security and in app HTML flexibility to increase. Without the baggage of govspeak options we could also just allow passing raw markdown direct to a component rather than having an app convert it as is done with govspeak. I've not taken these thoughts any further here though, I'll see how this pans out and if these are real needs. [1]: https://github.com/gjtorikian/commonmarker [2]: alphagov/govuk-frontend#778 [3]: https://github.com/alphagov/content-publisher/blob/654bf1a257e9a0de607e2c16b61754d55f5f8a72/app/helpers/application_helper.rb#L9
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We don't have confidence that the prose scope is well understood – without further research / better documentation it's safer to remove it for now.
If you are currently using prose-scope, you should revert to applying classes to individual headings, lists and hr elements.
In version 0.0.29-alpha release we have added the option to enable global link and paragraph styles. If you're using a recent version of the Private Beta Prototype kit, this is enabled by default.
Trello ticket: https://trello.com/c/v7tR99UO/955-remove-prose-scope